diff --git a/docs/api.md b/docs/api.md index 8a29e40..7d26d18 100644 --- a/docs/api.md +++ b/docs/api.md @@ -14,6 +14,10 @@ All described objects can be imported from the `xrlint.all` module. ::: xrlint.config.Config +## Class `ConfigList` + +::: xrlint.config.ConfigList + ## Class `RuleConfig` ::: xrlint.rule.RuleConfig @@ -33,3 +37,16 @@ All described objects can be imported from the `xrlint.all` module. ## Class `RuleContext` ::: xrlint.rule.RuleContext + +## Class `Result` + +::: xrlint.result.Result + +## Class `Message` + +::: xrlint.result.Message + +## Class `Suggestion` + +::: xrlint.result.Suggestion + diff --git a/mkdocs.yml b/mkdocs.yml index f20c19d..5e86b01 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -49,6 +49,7 @@ plugins: options: show_root_toc_entry: false show_root_heading: false - show_source: false + show_source: true heading_level: 3 annotations_path: brief + members_order: source diff --git a/tests/plugins/xcube/rules/test_spatial_dims_order.py b/tests/plugins/xcube/rules/test_cube_dims_order.py similarity index 79% rename from tests/plugins/xcube/rules/test_spatial_dims_order.py rename to tests/plugins/xcube/rules/test_cube_dims_order.py index 3274dcd..a98ad6e 100644 --- a/tests/plugins/xcube/rules/test_spatial_dims_order.py +++ b/tests/plugins/xcube/rules/test_cube_dims_order.py @@ -1,6 +1,6 @@ import numpy as np -from xrlint.plugins.xcube.rules.spatial_dims_order import SpatialDimsOrder +from xrlint.plugins.xcube.rules.cube_dims_order import CubeDimsOrder import xarray as xr @@ -34,24 +34,28 @@ def make_dataset(dims: tuple[str, str, str]): valid_dataset_1 = make_dataset(("time", "y", "x")) valid_dataset_2 = make_dataset(("time", "lat", "lon")) +valid_dataset_3 = make_dataset(("level", "y", "x")) invalid_dataset_1 = make_dataset(("time", "x", "y")) invalid_dataset_2 = make_dataset(("x", "y", "time")) invalid_dataset_3 = make_dataset(("time", "lon", "lat")) -invalid_dataset_4 = make_dataset(("lon", "lat", "time")) +invalid_dataset_4 = make_dataset(("lon", "lat", "level")) +invalid_dataset_5 = make_dataset(("x", "y", "level")) -SpatialDimsOrderTest = RuleTester.define_test( - "spatial-dims-order", - SpatialDimsOrder, +CubeDimsOrderTest = RuleTester.define_test( + "cube-dims-order", + CubeDimsOrder, valid=[ RuleTest(dataset=valid_dataset_1), RuleTest(dataset=valid_dataset_2), + RuleTest(dataset=valid_dataset_3), ], invalid=[ RuleTest(dataset=invalid_dataset_1), RuleTest(dataset=invalid_dataset_2), RuleTest(dataset=invalid_dataset_3), RuleTest(dataset=invalid_dataset_4), + RuleTest(dataset=invalid_dataset_5), ], ) diff --git a/tests/plugins/xcube/rules/test_lat_lon_naming.py b/tests/plugins/xcube/rules/test_lat_lon_naming.py new file mode 100644 index 0000000..06029f8 --- /dev/null +++ b/tests/plugins/xcube/rules/test_lat_lon_naming.py @@ -0,0 +1,65 @@ +import numpy as np + +from xrlint.plugins.xcube.rules.lat_lon_naming import LatLonNaming + +import xarray as xr + +from xrlint.testing import RuleTester, RuleTest + + +def make_dataset(lat_dim: str, lon_dim: str): + dims = ["time", lat_dim, lon_dim] + n = 3 + return xr.Dataset( + attrs=dict(title="v-data"), + coords={ + lon_dim: xr.DataArray( + np.linspace(0, 1, n), dims=lon_dim, attrs={"units": "m"} + ), + lat_dim: xr.DataArray( + np.linspace(0, 1, n), dims=lat_dim, attrs={"units": "m"} + ), + "time": xr.DataArray( + list(range(2010, 2010 + n)), dims="time", attrs={"units": "years"} + ), + }, + data_vars={ + "chl": xr.DataArray( + np.random.random((n, n, n)), dims=dims, attrs={"units": "mg/m^-3"} + ), + "tsm": xr.DataArray( + np.random.random((n, n, n)), dims=dims, attrs={"units": "mg/m^-3"} + ), + "avg_temp": xr.DataArray( + np.random.random(n), dims=dims[0], attrs={"units": "kelvin"} + ), + "mask": xr.DataArray(np.random.random((n, n)), dims=dims[-2:]), + }, + ) + + +valid_dataset_1 = make_dataset("lat", "lon") + +invalid_dataset_1 = make_dataset("lat", "long") +invalid_dataset_2 = make_dataset("lat", "longitude") +invalid_dataset_3 = make_dataset("lat", "Lon") + +invalid_dataset_4 = make_dataset("ltd", "lon") +invalid_dataset_5 = make_dataset("latitude", "lon") +invalid_dataset_6 = make_dataset("Lat", "lon") + +LatLonNamingTest = RuleTester.define_test( + "lat-lon-naming", + LatLonNaming, + valid=[ + RuleTest(dataset=valid_dataset_1), + ], + invalid=[ + RuleTest(dataset=invalid_dataset_1), + RuleTest(dataset=invalid_dataset_2), + RuleTest(dataset=invalid_dataset_3), + RuleTest(dataset=invalid_dataset_4), + RuleTest(dataset=invalid_dataset_5), + RuleTest(dataset=invalid_dataset_6), + ], +) diff --git a/tests/plugins/xcube/test_plugin.py b/tests/plugins/xcube/test_plugin.py index 3ecfe87..8a26768 100644 --- a/tests/plugins/xcube/test_plugin.py +++ b/tests/plugins/xcube/test_plugin.py @@ -18,7 +18,8 @@ def test_rules_complete(self): _plugin = export_plugin() self.assertEqual( { - "spatial-dims-order", + "cube-dims-order", + "lat-lon-naming", }, set(_plugin.rules.keys()), ) diff --git a/tests/test_linter.py b/tests/test_linter.py index 144c37b..f1df64b 100644 --- a/tests/test_linter.py +++ b/tests/test_linter.py @@ -33,7 +33,7 @@ def test_new_linter(self): self.assertEqual({CORE_PLUGIN_NAME, "xcube"}, set(linter.config.plugins.keys())) self.assertIsInstance(linter.config.rules, dict) self.assertIn("dataset-title-attr", linter.config.rules) - self.assertIn("xcube/spatial-dims-order", linter.config.rules) + self.assertIn("xcube/cube-dims-order", linter.config.rules) linter = new_linter(recommended=False) self.assertIsInstance(linter, xrl.Linter) diff --git a/tests/test_result.py b/tests/test_result.py index 87682d8..84d491d 100644 --- a/tests/test_result.py +++ b/tests/test_result.py @@ -2,7 +2,7 @@ from xrlint.config import Config from xrlint.plugin import Plugin, PluginMeta -from xrlint.result import get_rules_meta_for_results, Result, Message +from xrlint.result import get_rules_meta_for_results, Result, Message, Suggestion from xrlint.rule import RuleOp, RuleMeta @@ -76,3 +76,16 @@ def test_repr_html(self): self.assertIsInstance(html, str) self.assertIn("", html) self.assertIn("
", html) + + +class SuggestionTest(TestCase): + + # noinspection PyUnusedLocal + def test_from_value(self): + self.assertEqual( + Suggestion("Use xr.transpose()"), + Suggestion.from_value("Use xr.transpose()"), + ) + + suggestion = Suggestion("Use xr.transpose()") + self.assertIs(suggestion, Suggestion.from_value(suggestion)) diff --git a/xrlint/_linter/rule_ctx_impl.py b/xrlint/_linter/rule_ctx_impl.py index ba318a6..130b2c8 100644 --- a/xrlint/_linter/rule_ctx_impl.py +++ b/xrlint/_linter/rule_ctx_impl.py @@ -57,8 +57,11 @@ def report( message: str, *, fatal: bool | None = None, - suggestions: list[Suggestion] | None = None, + suggestions: list[Suggestion | str] | None = None, ): + suggestions = ( + [Suggestion.from_value(s) for s in suggestions] if suggestions else None + ) m = Message( message=message, fatal=fatal, diff --git a/xrlint/config.py b/xrlint/config.py index 208b158..fb5f20e 100644 --- a/xrlint/config.py +++ b/xrlint/config.py @@ -364,7 +364,7 @@ def from_value(cls, value: Any) -> "ConfigList": Args: value: A `ConfigList` object or `list` of values which can be - converted into `Config` objects. + converted into `Config` objects. Returns: A `ConfigList` object. """ diff --git a/xrlint/plugins/xcube/__init__.py b/xrlint/plugins/xcube/__init__.py index d3ec036..ff7e488 100644 --- a/xrlint/plugins/xcube/__init__.py +++ b/xrlint/plugins/xcube/__init__.py @@ -12,7 +12,7 @@ def export_plugin() -> Plugin: { "name": "recommended", "rules": { - f"xcube/{rule_id}": "error" for rule_id, rule in plugin.rules.items() + "xcube/cube-dims-order": "error", }, } ) diff --git a/xrlint/plugins/xcube/rules/cube_dims_order.py b/xrlint/plugins/xcube/rules/cube_dims_order.py new file mode 100644 index 0000000..fc7ea79 --- /dev/null +++ b/xrlint/plugins/xcube/rules/cube_dims_order.py @@ -0,0 +1,58 @@ +from xrlint.node import DataArrayNode +from xrlint.plugins.xcube.rules import plugin +from xrlint.result import Suggestion +from xrlint.rule import RuleOp, RuleContext + + +@plugin.define_rule( + "cube-dims-order", + version="1.0.0", + description=( + "Order of dimensions in spatio-temporal datacube variables" + " should be [time, ..., y, x]." + ), +) +class CubeDimsOrder(RuleOp): + def data_array(self, ctx: RuleContext, node: DataArrayNode): + if node.in_data_vars(): + dims = list(node.data_array.dims) + indexes = {d: i for i, d in enumerate(node.data_array.dims)} + + yx_names = None + if "x" in indexes and "y" in indexes: + yx_names = ["y", "x"] + elif "lon" in indexes and "lat" in indexes: + yx_names = ["lat", "lon"] + else: + # TODO: get yx_names/yx_indexes from grid-mapping + pass + if yx_names is None: + # This rule only applies to spatial dimensions + return + + t_name = None + if "time" in indexes: + t_name = "time" + + n = len(dims) + t_index = indexes[t_name] if t_name else None + y_index = indexes[yx_names[0]] + x_index = indexes[yx_names[1]] + + yx_ok = y_index == n - 2 and x_index == n - 1 + t_ok = t_index is None or t_index == 0 + if not yx_ok or not t_ok: + if t_index is None: + expected_dims = [d for d in dims if d not in yx_names] + yx_names + else: + expected_dims = ( + [t_name] + + [d for d in dims if d != t_name and d not in yx_names] + + yx_names + ) + # noinspection PyTypeChecker + ctx.report( + f"order of dimensions should be" + f" {','.join(expected_dims)}, but was {','.join(dims)}", + suggestions=["Use xarray.transpose(...) to reorder dimensions."], + ) diff --git a/xrlint/plugins/xcube/rules/lat_lon_naming.py b/xrlint/plugins/xcube/rules/lat_lon_naming.py new file mode 100644 index 0000000..e16213f --- /dev/null +++ b/xrlint/plugins/xcube/rules/lat_lon_naming.py @@ -0,0 +1,48 @@ +from xrlint.node import DatasetNode +from xrlint.plugins.xcube.rules import plugin +from xrlint.rule import RuleOp, RuleContext + +VALID_LON = "lon" +VALID_LAT = "lat" +INVALID_LONS = {"lng", "long", "longitude"} +INVALID_LATS = {"ltd", "latitude"} + + +@plugin.define_rule( + "lat-lon-naming", + version="1.0.0", + description=( + f"Latitude and longitude coordinates and dimensions" + f" should be called {VALID_LAT!r} and {VALID_LON!r}." + ), +) +class LatLonNaming(RuleOp): + def dataset(self, ctx: RuleContext, node: DatasetNode): + lon_ok = _check( + ctx, "variable", node.dataset.variables.keys(), INVALID_LONS, VALID_LON + ) + lat_ok = _check( + ctx, "variable", node.dataset.variables.keys(), INVALID_LATS, VALID_LAT + ) + if lon_ok and lat_ok: + # If variables have been reported, + # we should not need to report (their) coordinates + _check(ctx, "dimension", node.dataset.sizes.keys(), INVALID_LONS, VALID_LON) + _check(ctx, "dimension", node.dataset.sizes.keys(), INVALID_LATS, VALID_LAT) + + +def _check(ctx, names_name, names, invalid_names, valid_name): + names = [str(n) for n in names] # xarray keys are Hashable, not str + found_names = [ + n + for n in names + if (n.lower() in invalid_names) or (n.lower() == valid_name and n != valid_name) + ] + if found_names: + ctx.report( + f"The {names_name} {found_names[0]!r} should be named {valid_name!r}.", + suggestions=[f"Rename {names_name} to {valid_name!r}."], + ) + return False + else: + return True diff --git a/xrlint/plugins/xcube/rules/spatial_dims_order.py b/xrlint/plugins/xcube/rules/spatial_dims_order.py deleted file mode 100644 index 9527ad3..0000000 --- a/xrlint/plugins/xcube/rules/spatial_dims_order.py +++ /dev/null @@ -1,32 +0,0 @@ -from xrlint.node import DataArrayNode -from xrlint.plugins.xcube.rules import plugin -from xrlint.rule import RuleOp, RuleContext - - -@plugin.define_rule( - "spatial-dims-order", - version="1.0.0", - description="Spatial dimensions should have order [...,y,x].", -) -class SpatialDimsOrder(RuleOp): - def data_array(self, ctx: RuleContext, node: DataArrayNode): - if node.in_data_vars(): - dims = list(node.data_array.dims) - try: - yx_names = ["y", "x"] - yx_indexes = tuple(map(dims.index, yx_names)) - except ValueError: - try: - yx_names = ["lat", "lon"] - yx_indexes = tuple(map(dims.index, yx_names)) - except ValueError: - return - n = len(dims) - y_index, x_index = yx_indexes - if y_index != n - 2 or x_index != n - 1: - expected_dims = [d for d in dims if d not in yx_names] + yx_names - # noinspection PyTypeChecker - ctx.report( - f"order of dimensions should be" - f" {','.join(expected_dims)}, but was {','.join(dims)}" - ) diff --git a/xrlint/result.py b/xrlint/result.py index d2a6c02..1d36bdd 100644 --- a/xrlint/result.py +++ b/xrlint/result.py @@ -1,5 +1,5 @@ from dataclasses import dataclass, field -from typing import Literal, TYPE_CHECKING +from typing import Literal, TYPE_CHECKING, Any import html from tabulate import tabulate @@ -8,6 +8,7 @@ from xrlint.constants import SEVERITY_ERROR from xrlint.constants import SEVERITY_WARN from xrlint.util.formatting import format_problems +from xrlint.util.formatting import format_message_type_of from xrlint.util.todict import ToDictMixin if TYPE_CHECKING: # pragma: no cover @@ -31,6 +32,24 @@ class Suggestion(ToDictMixin): fix: EditInfo | None = None """Not used yet.""" + @classmethod + def from_value(cls, value: Any): + """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")) + @dataclass(kw_only=True) class Message(ToDictMixin): @@ -147,6 +166,14 @@ def _repr_html_(self) -> str: def get_rules_meta_for_results(results: list[Result]) -> dict[str, "RuleMeta"]: + """Get all rule metadata from the list of `results`. + + Args: + results: List of `Result` objects. + + Returns: + A dictionary that maps unique rule names to `RuleMeta` objects. + """ rules_meta = {} for result in results: for message in result.messages: diff --git a/xrlint/rule.py b/xrlint/rule.py index 5229c7f..41ad70f 100644 --- a/xrlint/rule.py +++ b/xrlint/rule.py @@ -40,7 +40,7 @@ def report( message: str, *, fatal: bool | None = None, - suggestions: list[Suggestion] | None = None, + suggestions: list[Suggestion | str] | None = None, ): """Report an issue. @@ -48,7 +48,8 @@ def report( message: mandatory message text fatal: True, if a fatal error is reported. suggestions: A list of suggestions for the user - on how to fix the reported issue. + on how to fix the reported issue. Items may + be of type `Suggestion` or `str`. """