Merge pull request #5 from bcdev/forman-more_xcube_rules

forman · web-flow · commit e2326298cb00 · 2024-12-31T12:13:36.000+01:00
More xcube rules
diff --git 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
@@ -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_cube_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
@@ -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
@@ -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
@@ -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
@@ -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("<table>", html)
         self.assertIn("</table>", 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/xrlint/result.py b/xrlint/result.py
diff --git a/xrlint/rule.py b/xrlint/rule.py

Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,8 @@ def test_rules_complete(self):`
`18`	`18`	`_plugin = export_plugin()`
`19`	`19`	`self.assertEqual(`
`20`	`20`	`{`
`21`		`- "spatial-dims-order",`
	`21`	`+ "cube-dims-order",`
	`22`	`+ "lat-lon-naming",`
`22`	`23`	`},`
`23`	`24`	`set(_plugin.rules.keys()),`
`24`	`25`	`)`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ def export_plugin() -> Plugin:`
`12`	`12`	`{`
`13`	`13`	`"name": "recommended",`
`14`	`14`	`"rules": {`
`15`		`- f"xcube/{rule_id}": "error" for rule_id, rule in plugin.rules.items()`
	`15`	`+ "xcube/cube-dims-order": "error",`
`16`	`16`	`},`
`17`	`17`	`}`
`18`	`18`	`)`