Merge pull request #12 from bcdev/forman-simple_format_outputs_immediately

"simple" formatter outputs immediately

Author: forman

CHANGES.md

@@ -2,7 +2,8 @@
 
 ## Version 0.1.0 (in development)
 
-- ...
+- XRLint CLI now outputs single results immediately to console,
+  instead only after all results have been collected.
 
 
 ## Early development snapshots

docs/todo.md

@@ -11,14 +11,15 @@
 ## Desired
 
 - project logo
+- if configuration for given FILE is empty,
+  report an error, see TODO in CLI main tests
+- rename `xrlint.cli.CliEngine` into `xrlint.cli.XRLint`
+  (with similar API as the `ESLint` class) and export it 
+  from `xrlint.all`. Value of `FILES` should be passed to
+  `verify_datasets()` methods.
 - use `RuleMeta.docs_url` in formatters to create links
 - implement xarray backend for xcube 'levels' format
   so can validate them too
-- CLI should output result for file immediately,
-  not only after all results have been collected
-- rename `xrlint.cli.CliEngine` into `xrlint.cli.XRLint`
-  (with similar API as the `ESLint` class) and export it 
-  from `xrlint.all`
 - add some more tests so we reach 99% coverage
 - support rule op args/kwargs schema validation
 - support CLI option `--print-config FILE`, see ESLint

tests/cli/test_main.py

@@ -11,6 +11,14 @@
 from xrlint.version import version
 from .helpers import text_file
 
+no_match_config_yaml = """
+- ignores: ['**/*.nc', '**/*.zarr']
+  rules:
+    dataset-title-attr: error
+"""
+
+no_match_config_yaml = "[]"
+
 
 # noinspection PyTypeChecker
 class CliMainTest(TestCase):
@@ -179,6 +187,14 @@ def test_files_with_format_option(self):
             self.assertIn('"results": [\n', result.output)
             self.assertEqual(0, result.exit_code)
 
+    def test_file_does_not_match(self):
+        with text_file(DEFAULT_CONFIG_FILE_YAML, no_match_config_yaml):
+            runner = CliRunner()
+            result = runner.invoke(main, ["test.zarr"])
+            # TODO: make this assertion work
+            # self.assertIn("No configuration matches this file.", result.output)
+            self.assertEqual(1, result.exit_code)
+
     def test_files_with_invalid_format_option(self):
         runner = CliRunner()
         result = runner.invoke(main, ["-f", "foo"] + self.files)

tests/formatters/test_markdown.py

@@ -2,28 +2,17 @@
 
 import pytest
 
-from xrlint.config import Config
 from xrlint.formatter import FormatterContext
 from xrlint.formatters.markdown import Markdown
-from xrlint.result import Message
-from xrlint.result import Result
+from .helpers import get_test_results
 
 
 class MarkdownTest(TestCase):
+    # noinspection PyMethodMayBeStatic
     def test_markdown(self):
         formatter = Markdown()
         with pytest.raises(NotImplementedError):
             formatter.format(
                 context=FormatterContext(),
-                results=[
-                    Result.new(
-                        Config(),
-                        file_path="test.nc",
-                        messages=[
-                            Message(message="what", rule_id="rule-1", severity=2),
-                            Message(message="is", fatal=True),
-                            Message(message="happening?", rule_id="rule-2", severity=1),
-                        ],
-                    )
-                ],
+                results=get_test_results(),
             )

tests/formatters/test_simple.py

@@ -21,7 +21,7 @@ class SimpleTest(TestCase):
     ]
 
     def test_no_color(self):
-        formatter = Simple(color_enabled=False)
+        formatter = Simple(styled=False)
         text = formatter.format(
             context=FormatterContext(),
             results=self.results,
@@ -30,7 +30,7 @@ def test_no_color(self):
         self.assertNotIn("\033]", text)
 
     def test_color(self):
-        formatter = Simple(color_enabled=True)
+        formatter = Simple(styled=True)
         text = formatter.format(
             context=FormatterContext(),
             results=self.results,

xrlint/cli/engine.py

@@ -1,5 +1,6 @@
 import os
-from collections.abc import Generator
+from collections.abc import Iterable
+from typing import Iterator
 
 import click
 import fsspec
@@ -39,7 +40,7 @@ def __init__(
         rule_specs: tuple[str, ...] = (),
         output_format: str = DEFAULT_OUTPUT_FORMAT,
         output_path: str | None = None,
-        color_enabled: bool = True,
+        styled: bool = True,
         files: tuple[str, ...] = (),
     ):
         self.no_default_config = no_default_config
@@ -48,7 +49,7 @@ def __init__(
         self.rule_specs = rule_specs
         self.output_format = output_format
         self.output_path = output_path
-        self.color_enabled = color_enabled
+        self.styled = styled
         self.files = files
 
     def load_config(self) -> ConfigList:
@@ -74,6 +75,7 @@ def load_config(self) -> ConfigList:
             for config_path in DEFAULT_CONFIG_FILES:
                 try:
                     config_list = read_config_list(config_path)
+                    break
                 except FileNotFoundError:
                     pass
                 except ConfigError as e:
@@ -92,16 +94,15 @@ def load_config(self) -> ConfigList:
 
         return ConfigList.from_value(configs)
 
-    def verify_datasets(self, config_list: ConfigList) -> list[Result]:
+    def verify_datasets(self, config_list: ConfigList) -> Iterator[Result]:
         global_filter = config_list.get_global_filter(default=DEFAULT_GLOBAL_FILTER)
-        results: list[Result] = []
+        linter = Linter()
         for file_path, is_dir in get_files(self.files, global_filter):
             config = config_list.compute_config(file_path)
             if config is not None:
-                linter = Linter(config=config)
-                result = linter.verify_dataset(file_path)
+                yield linter.verify_dataset(file_path, config=config)
             else:
-                result = Result.new(
+                yield Result.new(
                     config=config,
                     file_path=file_path,
                     messages=[
@@ -111,11 +112,8 @@ def verify_datasets(self, config_list: ConfigList) -> list[Result]:
                         )
                     ],
                 )
-            results.append(result)
 
-        return results
-
-    def format_results(self, results: list[Result]) -> str:
+    def format_results(self, results: Iterable[Result]) -> str:
         output_format = (
             self.output_format if self.output_format else DEFAULT_OUTPUT_FORMAT
         )
@@ -130,19 +128,23 @@ def format_results(self, results: list[Result]) -> str:
         # TODO: pass and validate format-specific args/kwargs
         #   against formatter.meta.schema
         if output_format == "simple":
-            formatter_kwargs = {"color_enabled": self.color_enabled}
+            formatter_kwargs = {
+                "styled": self.styled and self.output_path is None,
+                "output": self.output_path is None,
+            }
         else:
             formatter_kwargs = {}
         # noinspection PyArgumentList
         formatter_op = formatter.op_class(**formatter_kwargs)
         return formatter_op.format(FormatterContext(False), results)
 
     def write_report(self, report: str):
-        if not self.output_path:
-            print(report)
-        else:
+        if self.output_path:
             with fsspec.open(self.output_path, mode="w") as f:
                 f.write(report)
+        elif self.output_format != "simple":
+            # The simple formatters outputs incrementally to console
+            print(report)
 
     @classmethod
     def init_config_file(cls):
@@ -156,7 +158,7 @@ def init_config_file(cls):
 
 def get_files(
     file_paths: tuple[str, ...], global_filter: FileFilter
-) -> Generator[tuple[str, bool | None]]:
+) -> Iterator[tuple[str, bool | None]]:
     for file_path in file_paths:
         _fs, root = fsspec.url_to_fs(file_path)
         fs: fsspec.AbstractFileSystem = _fs

xrlint/cli/main.py

@@ -2,6 +2,8 @@
 
 import click
 
+from xrlint.cli.stats import Stats
+
 # Warning: do not import heavy stuff here, it can
 # slow down commands like "xrlint --help" otherwise.
 from xrlint.version import version
@@ -129,16 +131,18 @@ def main(
         files=files,
         output_format=output_format,
         output_path=output_file,
-        color_enabled=color_enabled,
+        styled=color_enabled,
     )
+    stats = Stats()
 
     config_list = cli_engine.load_config()
     results = cli_engine.verify_datasets(config_list)
+    results = stats.collect(results)
     report = cli_engine.format_results(results)
     cli_engine.write_report(report)
 
-    error_status = sum(r.error_count for r in results) > 0
-    max_warn_status = sum(r.warning_count for r in results) > max_warnings
+    error_status = stats.error_count > 0
+    max_warn_status = stats.warning_count > max_warnings
     if max_warn_status and not error_status:
         click.echo("maximum number of warnings exceeded.")
     if max_warn_status or error_status:

xrlint/cli/stats.py

@@ -0,0 +1,19 @@
+from collections.abc import Iterable
+from dataclasses import dataclass
+
+from xrlint.result import Result
+
+
+@dataclass()
+class Stats:
+    """Utility to collect simple statistics from results."""
+
+    error_count: int = 0
+    warning_count: int = 0
+
+    def collect(self, results: Iterable[Result]) -> Iterable[Result]:
+        """Collect statistics from `results`."""
+        for result in results:
+            self.error_count += result.error_count
+            self.warning_count += result.warning_count
+            yield result

xrlint/formatter.py

@@ -1,5 +1,5 @@
 from abc import abstractmethod, ABC
-from collections.abc import Mapping
+from collections.abc import Mapping, Iterable
 from dataclasses import dataclass
 from typing import Any, Callable, Type
 
@@ -22,13 +22,13 @@ class FormatterOp(ABC):
     def format(
         self,
         context: FormatterContext,
-        results: list[Result],
+        results: Iterable[Result],
     ) -> str:
         """Format the given results.
 
         Args:
             context: formatting context
-            results: the results to format
+            results: an iterable of results to format
         Returns:
             A text representing the results in a given format
         """

xrlint/formatters/html.py

@@ -1,3 +1,5 @@
+from collections.abc import Iterable
+
 from xrlint.formatter import FormatterOp, FormatterContext
 from xrlint.formatters import registry
 from xrlint.result import Result, get_rules_meta_for_results
@@ -22,35 +24,37 @@ def __init__(self, with_meta: bool = False):
     def format(
         self,
         context: FormatterContext,
-        results: list[Result],
+        results: Iterable[Result],
     ) -> str:
-        text = []
-        n = len(results)
-
-        text.append('<div role="results">\n')
-        text.append("<h3>Results</h3>\n")
-        for i, r in enumerate(results):
-            text.append('<div role="result">\n')
-            text.append(r.to_html())
-            text.append("</div>\n")
-            if i < n - 1:
-                text.append("<hr/>\n")
-        text.append("</div>\n")
+        results = list(results)  # get them all
+
+        text_parts = [
+            '<div role="results">\n',
+            "<h3>Results</h3>\n",
+        ]
+
+        for i, result in enumerate(results):
+            if i > 0:
+                text_parts.append("<hr/>\n")
+            text_parts.append('<div role="result">\n')
+            text_parts.append(result.to_html())
+            text_parts.append("</div>\n")
+        text_parts.append("</div>\n")
 
         if self.with_meta:
             rules_meta = get_rules_meta_for_results(results)
-            text.append('<div role="rules_meta">\n')
-            text.append("<h3>Rules</h3>\n")
+            text_parts.append('<div role="rules_meta">\n')
+            text_parts.append("<h3>Rules</h3>\n")
             for rm in rules_meta.values():
-                text.append(
+                text_parts.append(
                     f"<p>Rule <strong>{rm.name}</strong>, version {rm.version}</p>\n"
                 )
                 if rm.description:
-                    text.append(f"<p>{rm.description}</p>\n")
+                    text_parts.append(f"<p>{rm.description}</p>\n")
                 if rm.docs_url:
-                    text.append(
+                    text_parts.append(
                         f'<p><a href="{rm.docs_url}">Rule documentation</a></p>\n'
                     )
-            text.append("</div>\n")
+            text_parts.append("</div>\n")
 
-        return "".join(text)
+        return "".join(text_parts)