Merge pull request #37 from bcdev/forman-improve_config_mgt

Improve configuration management

Author: forman

CHANGES.md

@@ -23,6 +23,8 @@
     dictionary, or a name that refers to a named configuration of a plugin.
 
 - Other changes:
+  - Property `config` of `Linter` now returns a `ConfigList` instead 
+    of a `Config` object.  
   - Directories that are recognized by file patterns associated with a non-empty 
     configuration object are no longer recursively traversed.
   - Node path names now contain the dataset index if a file path 

notebooks/mkdataset.py

@@ -36,7 +36,7 @@ def make_dataset() -> xr.Dataset:
                 attrs={
                     "standard_name": "time",
                     "long_name": "time",
-                    "units": "days since 2020-01-01 utc",
+                    "units": "days since 2020-01-01 +0:00",
                     "calendar": "gregorian",
                 },
             ),
@@ -71,7 +71,7 @@ def make_dataset_with_issues() -> xr.Dataset:
     invalid_ds.x.attrs["axis"] = "x"
     del invalid_ds.y.attrs["standard_name"]
     invalid_ds.y.attrs["axis"] = "y"
-    invalid_ds.time.attrs["units"] = "days since 2020-01-01 ß0:000:00"
+    invalid_ds.time.attrs["units"] = "days since 2020-01-01 UTC"
     invalid_ds.attrs = {}
     invalid_ds.sst.attrs["units"] = 1
     invalid_ds["sst_avg"] = xr.DataArray(

notebooks/xrlint-linter.ipynb

@@ -5,6 +5,7 @@
 # noinspection PyProtectedMember
 from xrlint._linter.rulectx import RuleContextImpl
 from xrlint.config import Config
+from xrlint.constants import NODE_ROOT_NAME
 from xrlint.result import Message, Suggestion
 
 
@@ -31,6 +32,7 @@ def test_report(self):
             [
                 Message(
                     message="What the heck do you mean?",
+                    node_path=NODE_ROOT_NAME,
                     rule_id="no-xxx",
                     severity=2,
                     suggestions=[
@@ -39,6 +41,7 @@ def test_report(self):
                 ),
                 Message(
                     message="You said it.",
+                    node_path=NODE_ROOT_NAME,
                     rule_id="no-xxx",
                     severity=2,
                     fatal=True,

tests/_linter/test_rulectx.py

@@ -3,8 +3,8 @@
 
 import xarray as xr
 
-from xrlint.config import Config
-from xrlint.constants import CORE_PLUGIN_NAME
+from xrlint.config import Config, ConfigList
+from xrlint.constants import CORE_PLUGIN_NAME, NODE_ROOT_NAME
 from xrlint.linter import Linter, new_linter
 from xrlint.node import AttrNode, AttrsNode, DataArrayNode, DatasetNode
 from xrlint.plugin import new_plugin
@@ -16,36 +16,77 @@
 class LinterTest(TestCase):
     def test_default_config_is_empty(self):
         linter = Linter()
-        self.assertEqual(Config(), linter.config)
+        self.assertEqual(ConfigList(), linter.config)
 
     def test_new_linter(self):
-        import xrlint.all as xrl
-
         linter = new_linter()
-        self.assertIsInstance(linter, xrl.Linter)
-        self.assertIsInstance(linter.config.plugins, dict)
-        self.assertEqual({CORE_PLUGIN_NAME}, set(linter.config.plugins.keys()))
-        self.assertEqual(None, linter.config.rules)
-
-        linter = new_linter(config_name=None)
-        self.assertIsInstance(linter, xrl.Linter)
-        self.assertIsInstance(linter.config.plugins, dict)
-        self.assertEqual({CORE_PLUGIN_NAME}, set(linter.config.plugins.keys()))
-        self.assertEqual(None, linter.config.rules)
+        self.assertIsInstance(linter, Linter)
+        self.assertEqual(1, len(linter.config.configs))
+        config = linter.config.configs[0]
+        self.assertIsInstance(config.plugins, dict)
+        self.assertEqual({CORE_PLUGIN_NAME}, set(config.plugins.keys()))
+        self.assertEqual(None, config.rules)
 
+    def test_new_linter_recommended(self):
         linter = new_linter("recommended")
-        self.assertIsInstance(linter, xrl.Linter)
-        self.assertIsInstance(linter.config.plugins, dict)
-        self.assertEqual({CORE_PLUGIN_NAME}, set(linter.config.plugins.keys()))
-        self.assertIsInstance(linter.config.rules, dict)
-        self.assertIn("coords-for-dims", linter.config.rules)
+        self.assertIsInstance(linter, Linter)
+        self.assertEqual(2, len(linter.config.configs))
+        config0 = linter.config.configs[0]
+        config1 = linter.config.configs[1]
+        self.assertIsInstance(config0.plugins, dict)
+        self.assertEqual({CORE_PLUGIN_NAME}, set(config0.plugins.keys()))
+        self.assertIsInstance(config1.rules, dict)
+        self.assertIn("coords-for-dims", config1.rules)
 
+    def test_new_linter_all(self):
         linter = new_linter("all")
-        self.assertIsInstance(linter, xrl.Linter)
-        self.assertIsInstance(linter.config.plugins, dict)
-        self.assertEqual({CORE_PLUGIN_NAME}, set(linter.config.plugins.keys()))
-        self.assertIsInstance(linter.config.rules, dict)
-        self.assertIn("coords-for-dims", linter.config.rules)
+        self.assertIsInstance(linter, Linter)
+        self.assertEqual(2, len(linter.config.configs))
+        config0 = linter.config.configs[0]
+        config1 = linter.config.configs[1]
+        self.assertIsInstance(config0.plugins, dict)
+        self.assertEqual({CORE_PLUGIN_NAME}, set(config0.plugins.keys()))
+        self.assertIsInstance(config1.rules, dict)
+        self.assertIn("coords-for-dims", config1.rules)
+
+
+class LinterVerifyConfigTest(TestCase):
+    def test_config_with_config_list(self):
+        linter = new_linter()
+        result = linter.verify_dataset(
+            xr.Dataset(),
+            config=ConfigList.from_value([{"rules": {"no-empty-attrs": 2}}]),
+        )
+        self.assert_result_ok(result, "Missing metadata, attributes are empty.")
+
+    def test_config_with_list_of_config(self):
+        linter = new_linter()
+        result = linter.verify_dataset(
+            xr.Dataset(),
+            config=[{"rules": {"no-empty-attrs": 2}}],
+        )
+        self.assert_result_ok(result, "Missing metadata, attributes are empty.")
+
+    def test_config_with_config_obj(self):
+        linter = new_linter()
+        result = linter.verify_dataset(
+            xr.Dataset(),
+            config={"rules": {"no-empty-attrs": 2}},
+        )
+        self.assert_result_ok(result, "Missing metadata, attributes are empty.")
+
+    def test_no_config(self):
+        linter = Linter()
+        result = linter.verify_dataset(
+            xr.Dataset(),
+        )
+        self.assert_result_ok(result, "No configuration given or matches '<dataset>'.")
+
+    def assert_result_ok(self, result: Result, expected_message: str):
+        self.assertIsInstance(result, Result)
+        self.assertEqual(1, len(result.messages))
+        self.assertEqual(2, result.messages[0].severity)
+        self.assertEqual(expected_message, result.messages[0].message)
 
 
 class LinterVerifyTest(TestCase):
@@ -88,6 +129,8 @@ class MultiLevelDataset(ProcessorOp):
             def preprocess(
                 self, file_path: str, _opener_options: dict[str, Any]
             ) -> list[tuple[xr.Dataset, str]]:
+                if file_path == "bad.levels":
+                    raise OSError("bad checksum")
                 return [
                     (xr.Dataset(attrs={"title": "Level 0"}), file_path + "/0.zarr"),
                     (xr.Dataset(attrs={"title": "Level 1"}), file_path + "/1.zarr"),
@@ -99,7 +142,7 @@ def postprocess(
                 return messages[0] + messages[1]
 
         config = Config(plugins={"test": plugin})
-        self.linter = Linter(config=config)
+        self.linter = Linter(config)
         super().setUp()
 
     def test_rules_are_ok(self):
@@ -110,7 +153,7 @@ def test_rules_are_ok(self):
                 "data-var-dim-must-have-coord",
                 "dataset-without-data-vars",
             ],
-            list(self.linter._config.plugins["test"].rules.keys()),
+            list(self.linter.config.configs[0].plugins["test"].rules.keys()),
         )
 
     def test_linter_respects_rule_severity_error(self):
@@ -181,6 +224,23 @@ def test_linter_respects_rule_severity_off(self):
             result,
         )
 
+    def test_linter_recognized_unknown_rule(self):
+        result = self.linter.verify_dataset(
+            xr.Dataset(), rules={"test/dataset-is-fast": 2}
+        )
+        self.assertEqual(
+            [
+                Message(
+                    message="unknown rule 'test/dataset-is-fast'",
+                    rule_id="test/dataset-is-fast",
+                    node_path=NODE_ROOT_NAME,
+                    severity=2,
+                    fatal=True,
+                )
+            ],
+            result.messages,
+        )
+
     def test_linter_real_life_scenario(self):
         dataset = xr.Dataset(
             attrs={
@@ -208,11 +268,13 @@ def test_linter_real_life_scenario(self):
 
         result = self.linter.verify_dataset(
             dataset,
-            rules={
-                "test/no-space-in-attr-name": "error",
-                "test/no-empty-attrs": "warn",
-                "test/data-var-dim-must-have-coord": "error",
-                "test/dataset-without-data-vars": "warn",
+            config={
+                "rules": {
+                    "test/no-space-in-attr-name": "error",
+                    "test/no-empty-attrs": "warn",
+                    "test/data-var-dim-must-have-coord": "error",
+                    "test/dataset-without-data-vars": "warn",
+                },
             },
         )
         self.assertEqual(
@@ -262,15 +324,13 @@ def test_linter_real_life_scenario(self):
             result,
         )
 
-    def test_processor(self):
+    def test_processor_ok(self):
         result = self.linter.verify_dataset(
             "test.levels",
-            config=Config.from_value(
-                {
-                    "processor": "test/multi-level-dataset",
-                    "rules": {"test/dataset-without-data-vars": "warn"},
-                }
-            ),
+            config={
+                "processor": "test/multi-level-dataset",
+                "rules": {"test/dataset-without-data-vars": "warn"},
+            },
         )
 
         self.assertEqual(
@@ -290,3 +350,24 @@ def test_processor(self):
             ],
             result.messages,
         )
+
+    def test_processor_fail(self):
+        result = self.linter.verify_dataset(
+            "bad.levels",
+            config={
+                "processor": "test/multi-level-dataset",
+                "rules": {"test/dataset-without-data-vars": "warn"},
+            },
+        )
+
+        self.assertEqual(
+            [
+                Message(
+                    message="bad checksum",
+                    severity=2,
+                    fatal=True,
+                    node_path=NODE_ROOT_NAME,
+                )
+            ],
+            result.messages,
+        )

tests/test_linter.py

@@ -2,6 +2,7 @@
 from xrlint.rule import RuleConfig, RuleExit, RuleOp
 
 from .rulectx import RuleContextImpl
+from ..constants import NODE_ROOT_NAME
 
 
 def apply_rule(
@@ -33,9 +34,9 @@ def apply_rule(
                 DatasetNode(
                     parent=None,
                     path=(
-                        "dataset"
+                        NODE_ROOT_NAME
                         if context.file_index is None
-                        else f"dataset[{context.file_index}]"
+                        else f"{NODE_ROOT_NAME}[{context.file_index}]"
                     ),
                     dataset=context.dataset,
                 ),

xrlint/_linter/apply.py

@@ -4,7 +4,7 @@
 import xarray as xr
 
 from xrlint.config import Config
-from xrlint.constants import SEVERITY_ERROR
+from xrlint.constants import SEVERITY_ERROR, NODE_ROOT_NAME
 from xrlint.node import Node
 from xrlint.result import Message, Suggestion
 from xrlint.rule import RuleContext
@@ -66,7 +66,7 @@ def report(
             fatal=fatal,
             suggestions=suggestions,
             rule_id=self.rule_id,
-            node_path=self.node.path if self.node is not None else None,
+            node_path=self.node.path if self.node is not None else NODE_ROOT_NAME,
             severity=self.severity,
         )
         self.messages.append(m)