diff --git a/docs/todo.md b/docs/todo.md index 1ddf56b..df802f0 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -14,15 +14,17 @@ ## Desired - project logo -- add some more tests so we reach 99% coverage - apply rule op args/kwargs validation schema - provide core rule that checks for configurable list of std attributes - measure time it takes to open a dataset and pass time into rule context so we can write a configurable rule that checks the opening time -- allow outputting suggestions, if any, that emitted by some rules +- allow outputting suggestions, if any, that are emitted by some rules +- enhance styling of `Result` representation in Jupyter notebooks + (check if we can expand/collapse messages with suggestions) ## Nice to have +- add some more tests so we reach 100% coverage - support `autofix` feature - support `md` (markdown) output format - support formatter op args/kwargs and apply validation schema diff --git a/tests/formatters/test_simple.py b/tests/formatters/test_simple.py index 87a2bb9..05d5187 100644 --- a/tests/formatters/test_simple.py +++ b/tests/formatters/test_simple.py @@ -7,10 +7,10 @@ class SimpleTest(TestCase): - results = [ + errors_and_warnings = [ Result.new( Config(), - file_path="test.nc", + file_path="test1.nc", messages=[ Message(message="what", rule_id="rule-1", severity=2), Message(message="is", fatal=True), @@ -19,29 +19,65 @@ class SimpleTest(TestCase): ) ] + warnings_only = [ + Result.new( + Config(), + file_path="test2.nc", + messages=[ + Message(message="what", rule_id="rule-1", severity=1), + Message(message="happened?", rule_id="rule-2", severity=1), + ], + ) + ] + def test_no_color(self): formatter = Simple(styled=False) text = formatter.format( context=get_context(), - results=self.results, + results=self.errors_and_warnings, + ) + self.assert_output_1_ok(text) + self.assertNotIn("\033]", text) + + formatter = Simple(styled=False) + text = formatter.format( + context=get_context(), + results=self.warnings_only, ) - self.assert_output_ok(text) + self.assert_output_2_ok(text) self.assertNotIn("\033]", text) def test_color(self): formatter = Simple(styled=True) text = formatter.format( context=get_context(), - results=self.results, + results=self.errors_and_warnings, + ) + self.assert_output_1_ok(text) + self.assertIn("\033]", text) + + formatter = Simple(styled=True) + text = formatter.format( + context=get_context(), + results=self.warnings_only, ) - self.assert_output_ok(text) + self.assert_output_2_ok(text) self.assertIn("\033]", text) - def assert_output_ok(self, text): + def assert_output_1_ok(self, text): self.assertIsInstance(text, str) - self.assertIn("test.nc", text) + self.assertIn("test1.nc", text) self.assertIn("happening?", text) self.assertIn("error", text) self.assertIn("warn", text) self.assertIn("rule-1", text) self.assertIn("rule-2", text) + + def assert_output_2_ok(self, text): + self.assertIsInstance(text, str) + self.assertIn("test2.nc", text) + self.assertIn("happened?", text) + self.assertNotIn("error", text) + self.assertIn("warn", text) + self.assertIn("rule-1", text) + self.assertIn("rule-2", text) diff --git a/tests/test_config.py b/tests/test_config.py index 8fab8c3..0bda16b 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,14 +1,24 @@ +from typing import Any from unittest import TestCase import pytest +import xarray as xr -from xrlint.config import Config, ConfigList -from xrlint.rule import RuleConfig +from xrlint.config import Config, ConfigList, get_core_config +from xrlint.constants import CORE_PLUGIN_NAME +from xrlint.plugin import Plugin, new_plugin +from xrlint.processor import define_processor, ProcessorOp +from xrlint.result import Message +from xrlint.rule import RuleConfig, Rule from xrlint.util.filefilter import FileFilter # noinspection PyMethodMayBeStatic class ConfigTest(TestCase): + def test_class_props(self): + self.assertEqual("config", Config.value_name()) + self.assertEqual("Config | dict | None", Config.value_type_name()) + def test_defaults(self): config = Config() self.assertEqual(None, config.name) @@ -20,6 +30,50 @@ def test_defaults(self): self.assertEqual(None, config.plugins) self.assertEqual(None, config.rules) + def test_get_plugin(self): + config = get_core_config() + plugin = config.get_plugin(CORE_PLUGIN_NAME) + self.assertIsInstance(plugin, Plugin) + + with pytest.raises(ValueError, match="unknown plugin 'xcube'"): + config.get_plugin("xcube") + + def test_get_rule(self): + config = get_core_config() + rule = config.get_rule("flags") + self.assertIsInstance(rule, Rule) + + with pytest.raises(ValueError, match="unknown rule 'foo'"): + config.get_rule("foo") + + def test_get_processor_op(self): + class MyProc(ProcessorOp): + def preprocess( + self, file_path: str, opener_options: dict[str, Any] + ) -> list[tuple[xr.Dataset, str]]: + pass + + def postprocess( + self, messages: list[list[Message]], file_path: str + ) -> list[Message]: + pass + + processor = define_processor("myproc", op_class=MyProc) + config = Config( + plugins=dict( + myplugin=new_plugin("myplugin", processors=dict(myproc=processor)) + ) + ) + + processor_op = config.get_processor_op(MyProc()) + self.assertIsInstance(processor_op, MyProc) + + processor_op = config.get_processor_op("myplugin/myproc") + self.assertIsInstance(processor_op, MyProc) + + with pytest.raises(ValueError, match="unknown processor 'myplugin/myproc2'"): + config.get_processor_op("myplugin/myproc2") + def test_from_value_ok(self): self.assertEqual(Config(), Config.from_value(None)) self.assertEqual(Config(), Config.from_value({})) @@ -167,7 +221,7 @@ def test_compute_config(self): config_list = ConfigList( [ - Config(settings={"a": 1, "b": 1}), + Config(ignores=["**/*.yaml"], settings={"a": 1, "b": 1}), Config(files=["**/datacubes/*.zarr"], settings={"b": 2}), Config(files=["**/*.txt"], settings={"a": 2}), ] @@ -185,6 +239,12 @@ def test_compute_config(self): config_list.compute_config(file_path), ) + file_path = "s3://wq-services/datacubes/config.yaml" + self.assertEqual( + None, + config_list.compute_config(file_path), + ) + def test_split_global_filter(self): config_list = ConfigList( [ diff --git a/tests/test_operation.py b/tests/test_operation.py index 43c5fc5..7a0e286 100644 --- a/tests/test_operation.py +++ b/tests/test_operation.py @@ -32,7 +32,7 @@ def op_base_class(cls) -> Type[ThingOp]: return ThingOp @classmethod - def op_name(cls) -> str: + def value_name(cls) -> str: return "thing" @classmethod @@ -62,10 +62,9 @@ class MyThingOp3(ThingOp): class OperationTest(TestCase): - def test_defaults(self): + def test_class_props(self): self.assertEqual(OperationMeta, Operation.meta_class()) self.assertEqual(type, Operation.op_base_class()) - self.assertEqual("operation", Operation.op_name()) self.assertEqual("export_operation", Operation.op_import_attr_name()) self.assertEqual("operation", Operation.value_name()) self.assertEqual( diff --git a/tests/test_plugin.py b/tests/test_plugin.py index e909050..b9e46bd 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -10,6 +10,10 @@ class PluginTest(TestCase): + def test_class_props(self): + self.assertEqual("plugin", Plugin.value_name()) + self.assertEqual("Plugin | dict | str", Plugin.value_type_name()) + def test_new_plugin(self): plugin = new_plugin(name="hello", version="2.4.5") self.assertEqual(Plugin(meta=PluginMeta(name="hello", version="2.4.5")), plugin) @@ -53,6 +57,10 @@ class MyRule2(RuleOp): class PluginMetaTest(TestCase): + def test_class_props(self): + self.assertEqual("plugin_meta", PluginMeta.value_name()) + self.assertEqual("PluginMeta | dict", PluginMeta.value_type_name()) + def test_from_value(self): self.assertEqual( PluginMeta(name="p", ref="a.b.c:p"), diff --git a/tests/test_processor.py b/tests/test_processor.py index eab125d..6bc7898 100644 --- a/tests/test_processor.py +++ b/tests/test_processor.py @@ -9,7 +9,19 @@ from xrlint.result import Message +class ProcessorMetaTest(TestCase): + def test_class_props(self): + self.assertEqual("processor_meta", ProcessorMeta.value_name()) + self.assertEqual("ProcessorMeta | dict", ProcessorMeta.value_type_name()) + + class ProcessorTest(TestCase): + def test_class_props(self): + self.assertEqual("processor", Processor.value_name()) + self.assertEqual( + "Processor | Type[ProcessorOp] | dict | str", Processor.value_type_name() + ) + def test_define_processor(self): registry = {} diff --git a/tests/test_rule.py b/tests/test_rule.py index e794f6c..bbdc4ac 100644 --- a/tests/test_rule.py +++ b/tests/test_rule.py @@ -19,6 +19,12 @@ def export_rule(): class RuleTest(TestCase): + def test_class_props(self): + self.assertIs(RuleMeta, Rule.meta_class()) + self.assertIs(RuleOp, Rule.op_base_class()) + self.assertEqual("rule", Rule.value_name()) + self.assertEqual("Rule | Type[RuleOp] | dict | str", Rule.value_type_name()) + def test_from_value_ok_rule(self): rule = export_rule() rule2 = Rule.from_value(rule) @@ -72,6 +78,10 @@ class MyRule3(RuleOp): class RuleMetaTest(unittest.TestCase): + def test_class_props(self): + self.assertEqual("rule_meta", RuleMeta.value_name()) + self.assertEqual("RuleMeta | dict", RuleMeta.value_type_name()) + def test_from_value(self): rule_meta = RuleMeta.from_value( { @@ -133,6 +143,10 @@ def test_fail(self): class RuleConfigTest(TestCase): + def test_class_props(self): + self.assertEqual("rule_config", RuleConfig.value_name()) + self.assertEqual("int | str | list", RuleConfig.value_type_name()) + def test_defaults(self): rule_config = RuleConfig(1) self.assertEqual(1, rule_config.severity) @@ -147,6 +161,10 @@ def test_from_value_ok(self): self.assertEqual(RuleConfig(1), RuleConfig.from_value("warn")) self.assertEqual(RuleConfig(2), RuleConfig.from_value("error")) self.assertEqual(RuleConfig(2), RuleConfig.from_value(["error"])) + # YAML "on"/"off" literals + self.assertEqual(RuleConfig(0), RuleConfig.from_value(False)) + self.assertEqual(RuleConfig(1), RuleConfig.from_value(True)) + self.assertEqual( RuleConfig(1, ("never",)), RuleConfig.from_value(["warn", "never"]) ) @@ -175,17 +193,19 @@ def test_from_value_ok(self): ) # noinspection PyMethodMayBeStatic - def test_from_value_fails(self): + def test_from_value_fail(self): with pytest.raises( TypeError, - match="rule configuration must be of type int|str|tuple|list, but got None", + match=r"rule_config must be of type int \| str \| list, but got None", ): RuleConfig.from_value(None) + with pytest.raises( ValueError, match="severity must be one of 'error', 'warn', 'off', 2, 1, 0, but got 4", ): RuleConfig.from_value(4) + with pytest.raises( ValueError, match=( @@ -194,3 +214,9 @@ def test_from_value_fails(self): ), ): RuleConfig.from_value("debug") + + with pytest.raises( + ValueError, + match="rule_config must not be empty", + ): + RuleConfig.from_value([]) diff --git a/tests/util/test_filefilter.py b/tests/util/test_filefilter.py index 4db4ae0..2811df2 100644 --- a/tests/util/test_filefilter.py +++ b/tests/util/test_filefilter.py @@ -62,9 +62,26 @@ def test_accept(self): self.assertEqual(True, file_filter.accept("test-1.zarr")) self.assertEqual(True, file_filter.accept("test-2.zarr")) + # negated ignores: file_filter = FileFilter.from_patterns( ["**/*.zarr"], ["**/temp/*", "!**/temp/x.zarr"] ) self.assertEqual(True, file_filter.accept("./test.zarr")) self.assertEqual(True, file_filter.accept("./temp/x.zarr")) self.assertEqual(False, file_filter.accept("./temp/y.zarr")) + + # negated ignores: + file_filter = FileFilter.from_patterns([], ["**/temp/*", "!**/temp/*.zarr"]) + self.assertEqual(True, file_filter.accept("./ds.nc")) + self.assertEqual(True, file_filter.accept("./temp/ds.zarr")) + self.assertEqual(True, file_filter.accept("./temp/test/ds.zarr")) + self.assertEqual(False, file_filter.accept("./temp/ds.nc")) + self.assertEqual(False, file_filter.accept("./temp/README.md")) + + # just ignores: + file_filter = FileFilter.from_patterns([], ["**/*.json", "**/*.yaml"]) + self.assertEqual(True, file_filter.accept("./test.nc")) + self.assertEqual(True, file_filter.accept("./temp/x.zarr")) + self.assertEqual(False, file_filter.accept("c.yaml")) + self.assertEqual(False, file_filter.accept("test/config.json")) + self.assertEqual(False, file_filter.accept("test/config.yaml")) diff --git a/tests/util/test_importutil.py b/tests/util/test_importutil.py index e5894e5..bd32e12 100644 --- a/tests/util/test_importutil.py +++ b/tests/util/test_importutil.py @@ -1,7 +1,13 @@ from unittest import TestCase +import pytest + from xrlint.plugin import Plugin -from xrlint.util.importutil import import_submodules, import_value +from xrlint.util.importutil import import_submodules, import_value, ValueImportError + + +def get_foo(): + return 42 class ImportSubmodulesTest(TestCase): @@ -24,9 +30,40 @@ def test_import_submodules(self): set(modules), ) - def test_import_exported_value(self): + +class ImportValueTest(TestCase): + def test_import_exported_value_ok(self): plugin, plugin_ref = import_value( "xrlint.plugins.core", "export_plugin", factory=Plugin.from_value ) self.assertIsInstance(plugin, Plugin) self.assertEqual("xrlint.plugins.core:export_plugin", plugin_ref) + + def test_import_exported_value_ok_no_factory(self): + value, value_ref = import_value( + "tests.util.test_importutil:get_foo", + ) + self.assertEqual(value, 42) + self.assertEqual("tests.util.test_importutil:get_foo", value_ref) + + # noinspection PyMethodMayBeStatic + def test_import_exported_value_fail(self): + with pytest.raises( + ValueImportError, + match=( + r"value of tests.util.test_importutil:get_foo\(\)" + r" must be of type float, but got int" + ), + ): + import_value("tests.util.test_importutil:get_foo", expected_type=float) + + # noinspection PyMethodMayBeStatic + def test_import_exported_value_import_error(self): + with pytest.raises( + ValueImportError, + match=( + "failed to import value from 'tests.util.test_baz:get_foo':" + " No module named 'tests.util.test_baz'" + ), + ): + import_value("tests.util.test_baz:get_foo") diff --git a/xrlint/config.py b/xrlint/config.py index ee294d4..ccf87ec 100644 --- a/xrlint/config.py +++ b/xrlint/config.py @@ -276,10 +276,11 @@ def _from_none(cls, value_name: str) -> "Config": @classmethod def forward_refs(cls) -> dict[str, type]: from xrlint.plugin import Plugin - from xrlint.processor import ProcessorOp + from xrlint.processor import Processor, ProcessorOp from xrlint.rule import Rule, RuleConfig return { + "Processor": Processor, "ProcessorOp": ProcessorOp, "Plugin": Plugin, "Rule": Rule, diff --git a/xrlint/formatter.py b/xrlint/formatter.py index da67ea7..5293388 100644 --- a/xrlint/formatter.py +++ b/xrlint/formatter.py @@ -80,7 +80,7 @@ def op_base_class(cls) -> Type: return FormatterOp @classmethod - def op_name(cls) -> str: + def value_name(cls) -> str: return "formatter" diff --git a/xrlint/operation.py b/xrlint/operation.py index 87332bd..79a1005 100644 --- a/xrlint/operation.py +++ b/xrlint/operation.py @@ -90,7 +90,7 @@ def _from_class(cls, value: Type, value_name: str) -> "Operation": meta = op_class.meta except AttributeError: raise ValueError( - f"missing {cls.op_name()} metadata, apply define_{cls.op_name()}()" + f"missing {cls.value_name()} metadata, apply define_{cls.value_name()}()" f" to class {op_class.__name__}" ) # noinspection PyArgumentList @@ -114,7 +114,7 @@ def op_import_attr_name(cls) -> str: """Get the default name for the attribute that is used to import instances of this class from modules. """ - return f"export_{cls.op_name()}" + return f"export_{cls.value_name()}" @classmethod def meta_class(cls) -> Type: @@ -131,16 +131,12 @@ def op_base_class(cls) -> Type: return type @classmethod - def op_name(cls) -> str: + def value_name(cls) -> str: """Get a name that describes the operation, e.g., "rule", "processor", "formatter". """ return "operation" - @classmethod - def value_name(cls) -> str: - return cls.op_name() - @classmethod def value_type_name(cls) -> str: return f"{cls.__name__} | Type[{cls.op_base_class().__name__}] | dict | str" @@ -158,7 +154,9 @@ def define_operation( meta_kwargs = meta_kwargs or {} def _define_op(_op_class: Type, decorated=True) -> Type | "Operation": - cls._assert_op_class_ok(f"decorated {cls.op_name()} component", _op_class) + cls._assert_op_class_ok( + f"decorated {cls.value_name()} component", _op_class + ) name = meta_kwargs.pop("name", None) if not name: diff --git a/xrlint/plugin.py b/xrlint/plugin.py index a7cab18..670ef7c 100644 --- a/xrlint/plugin.py +++ b/xrlint/plugin.py @@ -26,6 +26,10 @@ class PluginMeta(MappingConstructible, JsonSerializable): Must have the form ":", if given. """ + @classmethod + def value_name(cls) -> str: + return "plugin_meta" + @classmethod def value_type_name(cls) -> str: return "PluginMeta | dict" @@ -128,6 +132,10 @@ def _from_str(cls, value: str, value_name: str) -> "Plugin": plugin.meta.ref = plugin_ref return plugin + @classmethod + def value_name(cls) -> str: + return "plugin" + @classmethod def value_type_name(cls) -> str: return "Plugin | dict | str" diff --git a/xrlint/processor.py b/xrlint/processor.py index f45f554..f763c00 100644 --- a/xrlint/processor.py +++ b/xrlint/processor.py @@ -65,6 +65,10 @@ class ProcessorMeta(OperationMeta): Must have the form ":", if given. """ + @classmethod + def value_name(cls) -> str: + return "processor_meta" + @classmethod def value_type_name(cls) -> str: return "ProcessorMeta | dict" @@ -95,7 +99,7 @@ def op_base_class(cls) -> Type: return ProcessorOp @classmethod - def op_name(cls) -> str: + def value_name(cls) -> str: return "processor" diff --git a/xrlint/result.py b/xrlint/result.py index 943fa5a..0614506 100644 --- a/xrlint/result.py +++ b/xrlint/result.py @@ -1,7 +1,7 @@ import html from collections.abc import Iterable from dataclasses import dataclass, field -from typing import TYPE_CHECKING, Any, Literal, Union +from typing import TYPE_CHECKING, Literal, Union from tabulate import tabulate @@ -11,7 +11,8 @@ SEVERITY_ERROR, SEVERITY_WARN, ) -from xrlint.util.formatting import format_message_type_of, format_problems +from xrlint.util.constructible import ValueConstructible +from xrlint.util.formatting import format_problems from xrlint.util.serializable import JsonSerializable if TYPE_CHECKING: # pragma: no cover @@ -25,7 +26,7 @@ class EditInfo(JsonSerializable): @dataclass(frozen=True) -class Suggestion(JsonSerializable): +class Suggestion(ValueConstructible, JsonSerializable): desc: str """Description of the suggestion.""" @@ -36,23 +37,8 @@ class Suggestion(JsonSerializable): """Not used yet.""" @classmethod - def from_value(cls, value: Any, name: str | None = None) -> "Suggestion": - """Convert given `value` into a `Suggestion` object. - - If `value` is already a `Suggestion` then it is returned as-is. - - Args: - value: A `Suggestion` object or a `str` containing the - suggestion text. - - Returns: - A `Suggestion` object. - """ - if isinstance(value, Suggestion): - return value - if isinstance(value, str): - return Suggestion(value) - raise TypeError(format_message_type_of("value", value, "Suggestion|str")) + def _from_str(cls, value: str, value_name: str | None = None) -> "Suggestion": + return Suggestion(value) @dataclass() diff --git a/xrlint/rule.py b/xrlint/rule.py index 56894ef..6ade722 100644 --- a/xrlint/rule.py +++ b/xrlint/rule.py @@ -176,6 +176,10 @@ class RuleMeta(OperationMeta): by the rule’s implementation and its configured severity. """ + @classmethod + def value_name(cls) -> str: + return "rule_meta" + @classmethod def value_type_name(cls) -> str: return "RuleMeta | dict" @@ -211,7 +215,7 @@ def op_base_class(cls) -> Type: return RuleOp @classmethod - def op_name(cls) -> str: + def value_name(cls) -> str: return "rule" @@ -289,7 +293,7 @@ def _from_sequence(cls, value: Sequence, value_name: str) -> "RuleConfig": @classmethod def value_name(cls) -> str: - return "rule configuration" + return "rule_config" @classmethod def value_type_name(cls) -> str: diff --git a/xrlint/util/filefilter.py b/xrlint/util/filefilter.py index cad7da4..e295b67 100644 --- a/xrlint/util/filefilter.py +++ b/xrlint/util/filefilter.py @@ -42,23 +42,24 @@ def merge(self, file_filter: "FileFilter") -> "FileFilter": ) def accept(self, file_path) -> bool: - included = False - for m in self.files: - if m.match(file_path): - included = True - break - if not included: - return False + if self.files: + included = False + for p in self.files: + if p.match(file_path): + included = True + break + if not included: + return False excluded = False - for m in self.ignores: - if not m.negate: + for p in self.ignores: + if not p.negate: if excluded: # Already excluded, no need to check further return False - excluded = m.match(file_path) + excluded = p.match(file_path) else: - if excluded and m.match(file_path): + if excluded and p.match(file_path): # Negate the old excluded status on match excluded = False diff --git a/xrlint/util/filepattern.py b/xrlint/util/filepattern.py index 6cbe608..b90836c 100644 --- a/xrlint/util/filepattern.py +++ b/xrlint/util/filepattern.py @@ -97,7 +97,7 @@ def __eq__(self, other): return False return self.pattern == other.pattern - def __neq__(self, other): + def __ne__(self, other): return not self.__eq__(other) def __hash__(self): diff --git a/xrlint/util/importutil.py b/xrlint/util/importutil.py index 48ccd39..fbb35d5 100644 --- a/xrlint/util/importutil.py +++ b/xrlint/util/importutil.py @@ -7,8 +7,7 @@ def import_submodules(package_name: str, dry_run: bool = False) -> list[str]: package = importlib.import_module(package_name) - if not hasattr(package, "__path__"): - return [] + assert hasattr(package, "__path__") package_path = pathlib.Path(package.__path__[0]) @@ -98,7 +97,8 @@ def import_value( f" not found in module {module_name!r}" ) - if not constant and callable(attr_value): + should_invoke = not constant and callable(attr_value) + if should_invoke: # We don't catch exceptions here, # because they occur in user land. # noinspection PyCallingNonCallable @@ -116,7 +116,11 @@ def import_value( if expected_type is not None and not isinstance(exported_value, expected_type): raise ValueImportError( - format_message_type_of(module_ref, exported_value, expected_type) + format_message_type_of( + f"value of {module_ref}{('()' if should_invoke else '')}", + exported_value, + expected_type, + ) ) return exported_value, module_ref