Fix issues with __line__ in config

caput/config.py

@@ -183,8 +183,10 @@ def read_config(self, config, compare_keys=False, use_defaults=True):
         config : dict
             Dictionary of configuration values.
         compare_keys : bool or list[str]
-            If True, a CaputConfigError is raised if there are unused keys in the config dictionary.
-            If a list of strings is given, any unused keys except the ones in the list lead to a CaputConfigError.
+            If True, a CaputConfigError is raised if there are unused keys in the
+            config dictionary.
+            If a list of strings is given, any unused keys except the ones in the
+            list lead to a CaputConfigError.
         use_defaults : bool
             If False, a CaputConfigError is raised if a property is not defined by
             the config dictionary
@@ -225,8 +227,8 @@ def read_config(self, config, compare_keys=False, use_defaults=True):
     def _finalise_config(self):
         """Finish up the configuration.
 
-        To be overriden in subclasses if we need to perform some processing
-        post configutation.
+        To be overridden in subclasses if we need to perform some processing
+        post configuration.
         """
         pass
 
@@ -423,8 +425,8 @@ def logging_config(default={}):
     """
     A Property type that validates the caput logging config.
 
-    Allows the type to be either a string (for backward compatibility) or a dict setting log levels
-    per module.
+    Allows the type to be either a string (for backward compatibility) or a dict
+    setting log levels per module.
 
     Parameters
     ----------
@@ -454,15 +456,12 @@ def _prop(config):
                 "'{}'.".format(type(config.__name__))
             )
 
-        # check entries, get module names and warn for duplicates when sorting into new dict
+        # check entries, get module names and warn for duplicates when sorting into new
+        # dict
         checked_config = {}
         loglevels = ["DEBUG", "INFO", "WARNING", "ERROR", "NOTSET"]
         for key, level in config.items():
 
-            # ignore hint at yaml file line number
-            if key == "__line__":
-                continue
-
             level = level.upper()
             if level not in loglevels:
                 raise ValueError(
@@ -474,8 +473,8 @@ def _prop(config):
             already_set_to = checked_config.get(key, None)
             if already_set_to is not None and already_set_to != level:
                 logger.warning(
-                    "Setting log level for {} to {}, but is already set to {}. The old value will "
-                    "get ignored.".format(key, level, already_set_to)
+                    f"Setting log level for {key} to {level}, but is already set "
+                    f"to {already_set_to}. The old value will get ignored."
                 )
             checked_config[key] = level
         return checked_config
@@ -485,17 +484,26 @@ def _prop(config):
     return prop
 
 
+class _line_dict(dict):
+    """A private dict subclass that also stores line numbers for debugging."""
+
+    __line__ = None
+
+
 class SafeLineLoader(SafeLoader):
     """
     YAML loader that tracks line numbers.
 
-    Adds the line number information to every YAML block. This is useful for debugging and to describe linting errors.
+    Adds the line number information to every YAML block. This is useful for
+    debugging and to describe linting errors.
     """
 
     def construct_mapping(self, node, deep=False):
         mapping = super(SafeLineLoader, self).construct_mapping(node, deep=deep)
+        mapping = _line_dict(mapping)
+
         # Add 1 so numbering starts at 1
-        mapping["__line__"] = node.start_mark.line + 1
+        mapping.__line__ = node.start_mark.line + 1
         return mapping
 
 
@@ -510,15 +518,15 @@ class CaputConfigError(Exception):
     file_ : str
         Configuration file name (optional)
     location : dict
-        If using :class:`SafeLineLoader` is used, a dict created by that can be passed in here to report the line number
-        where the error occurred.
+        If using :class:`SafeLineLoader` is used, a dict created by that can be
+        passed in here to report the line number where the error occurred.
     """
 
     def __init__(self, message, file_=None, location=None):
         self.message = message
         self.file = file_
-        if isinstance(location, dict):
-            self.line = location.get("__line__", None)
+        if isinstance(location, _line_dict):
+            self.line = location.__line__
         else:
             self.line = None
 

caput/pipeline.py

@@ -672,7 +672,7 @@ def _setup_task(self, task_spec):
 
         # Check that only the expected keys are in the task spec.
         for key in task_spec.keys():
-            if key not in ["type", "params", "requires", "in", "out", "__line__"]:
+            if key not in ["type", "params", "requires", "in", "out"]:
                 raise config.CaputConfigError(
                     "Task got an unexpected key '{}' in 'tasks' list.".format(key)
                 )
@@ -882,9 +882,7 @@ def cacheable(self):
     def _from_config(cls, config):
         self = cls.__new__(cls)
         # Check for unused keys, but ignore the ones not put there by the user.
-        self.read_config(
-            config, compare_keys=["versions", "pipeline_config", "__line__"]
-        )
+        self.read_config(config, compare_keys=["versions", "pipeline_config"])
         self.__init__()
         return self
 

caput/tests/test_config.py

@@ -1,10 +1,12 @@
-import unittest
-
 import pickle
 
+import pytest
+import yaml
+
 from caput import config
 
 
+### Test classes
 class Person(config.Reader):
     name = config.Property(default="Bill", proptype=str)
     age = config.Property(default=26, proptype=float, key="ageinyears")
@@ -21,73 +23,108 @@ class ListTypeTests(Person):
     list_type = config.list_type(type_=int)
 
 
-class TestConfig(unittest.TestCase):
+class DictTypeTests(config.Reader):
+    dict_config = config.Property(proptype=dict)
+
+
+### Test data dict
+testdict = {"name": "Richard", "ageinyears": 40, "petname": "Sooty"}
+
+
+### Tests
+def test_default_params():
+
+    person1 = Person()
+
+    assert person1.name == "Bill"
+    assert person1.age == 26.0
+    assert isinstance(person1.age, float)
+
+
+def test_set_params():
+
+    person = Person()
+    person.name = "Mick"
+
+    assert person.name == "Mick"
+
+
+def test_read_config():
+
+    person = Person()
+    person.read_config(testdict)
+
+    assert person.name == "Richard"
+    assert person.age == 40.0
 
-    testdict = {"name": "Richard", "ageinyears": 40, "petname": "Sooty"}
 
-    def test_default_params(self):
+def test_inherit_read_config():
 
-        person1 = Person()
+    person = PersonWithPet()
+    person.read_config(testdict)
 
-        self.assertEqual(person1.name, "Bill")
-        self.assertEqual(person1.age, 26.0)
-        self.assertIsInstance(person1.age, float)
+    assert person.name == "Richard"
+    assert person.age == 40.0
+    assert person.petname == "Sooty"
 
-    def test_set_params(self):
 
-        person = Person()
-        person.name = "Mick"
+def test_pickle():
 
-        self.assertEqual(person.name, "Mick")
+    person = PersonWithPet()
+    person.read_config(testdict)
+    person2 = pickle.loads(pickle.dumps(person))
 
-    def test_read_config(self):
+    assert person2.name == "Richard"
+    assert person2.age == 40.0
+    assert person2.petname == "Sooty"
 
-        person = Person()
-        person.read_config(self.testdict)
 
-        self.assertEqual(person.name, "Richard")
-        self.assertEqual(person.age, 40.0)
+def test_list_type():
 
-    def test_inherit_read_config(self):
+    lt = ListTypeTests()
 
-        person = PersonWithPet()
-        person.read_config(self.testdict)
+    with pytest.raises(config.CaputConfigError):
+        lt.read_config({"list_max_length": [1, 3, 4]})
 
-        self.assertEqual(person.name, "Richard")
-        self.assertEqual(person.age, 40.0)
-        self.assertEqual(person.petname, "Sooty")
+    # Should work fine
+    lt = ListTypeTests()
+    lt.read_config({"list_max_length": [1, 2]})
 
-    def test_pickle(self):
+    with pytest.raises(config.CaputConfigError):
+        lt.read_config({"list_exact_length": [3]})
 
-        person = PersonWithPet()
-        person.read_config(self.testdict)
-        person2 = pickle.loads(pickle.dumps(person))
+    # Work should fine
+    lt = ListTypeTests()
+    lt.read_config({"list_exact_length": [1, 2]})
 
-        self.assertEqual(person2.name, "Richard")
-        self.assertEqual(person2.age, 40.0)
-        self.assertEqual(person2.petname, "Sooty")
+    with pytest.raises(config.CaputConfigError):
+        lt.read_config({"list_type": ["hello"]})
 
-    def test_list_type(self):
+    # Work should fine
+    lt = ListTypeTests()
+    lt.read_config({"list_type": [1, 2]})
 
-        lt = ListTypeTests()
 
-        with self.assertRaises(config.CaputConfigError):
-            lt.read_config({"list_max_length": [1, 3, 4]})
+def test_no_line():
+    # This tests that dicts get set as config parameters as expected, and covers a flaw
+    # in an earlier version of the linting code where `__line__` keys were getting
+    # inserted into dict types config properties
 
-        # Should work fine
-        lt = ListTypeTests()
-        lt.read_config({"list_max_length": [1, 2]})
+    dt = DictTypeTests()
 
-        with self.assertRaises(config.CaputConfigError):
-            lt.read_config({"list_exact_length": [3]})
+    # Test with an empty dict
+    yaml_str = yaml.dump({"dict_config": {}})
+    yaml_params = yaml.load(yaml_str, Loader=config.SafeLineLoader)
+    dt.read_config(yaml_params)
 
-        # Work should fine
-        lt = ListTypeTests()
-        lt.read_config({"list_exact_length": [1, 2]})
+    assert len(dt.dict_config) == 0
+    assert type(dt.dict_config) == dict
 
-        with self.assertRaises(config.CaputConfigError):
-            lt.read_config({"list_type": ["hello"]})
+    # Test with a non-empty dict
+    yaml_str = yaml.dump({"dict_config": {"a": 3}})
+    yaml_params = yaml.load(yaml_str, Loader=config.SafeLineLoader)
+    dt.read_config(yaml_params)
 
-        # Work should fine
-        lt = ListTypeTests()
-        lt.read_config({"list_type": [1, 2]})
+    assert len(dt.dict_config) == 1
+    assert type(dt.dict_config) == dict
+    assert dt.dict_config["a"] == 3

caput/tests/test_metadata.py

@@ -21,8 +21,6 @@ def test_default_params(self):
         self.assertDictEqual(man.all_tasks_params["versions"], {})
         # remove line numbers
         pipeline_config = man.all_tasks_params["pipeline_config"]
-        del pipeline_config["__line__"]
-        del pipeline_config["pipeline"]["__line__"]
         self.assertDictEqual(
             pipeline_config,
             yaml.load(testconfig, Loader=yaml.SafeLoader),
@@ -50,8 +48,6 @@ def test_metadata_params(self):
 
         # remove line numbers
         pipeline_config = man.all_tasks_params["pipeline_config"]
-        del pipeline_config["__line__"]
-        del pipeline_config["pipeline"]["__line__"]
         self.assertDictEqual(
             pipeline_config,
             yaml.load(testconfig, Loader=yaml.SafeLoader),