refactor(s2): validate CRS/EPSG and bbox attrs via typed constructors (#180 parity)

src/eopf_geozarr/conversion/geozarr.py

@@ -44,6 +44,7 @@
     StandardXCoordAttrsJSON,
     StandardYCoordAttrsJSON,
     XarrayEncodingJSON,
+    make_epsg_code,
 )
 
 from . import fs_utils, utils
@@ -177,11 +178,10 @@ def setup_datatree_metadata_geozarr_spec_compliant(
     """
     geozarr_groups: dict[str, xr.Dataset] = {}
     grid_mapping_var_name = "spatial_ref"
-    epsg_CPM_260 = dt.attrs.get("other_metadata", {}).get(
+    raw_epsg_cpm_260 = dt.attrs.get("other_metadata", {}).get(
         "horizontal_CRS_code", dt.attrs.get("other_metadata", {}).get("horizontal_crs_code", None)
     )
-    if epsg_CPM_260 is not None:
-        epsg_CPM_260 = epsg_CPM_260.split(":")[-1]
+    epsg_CPM_260 = make_epsg_code(raw_epsg_cpm_260) if raw_epsg_cpm_260 is not None else None
 
     for key in groups:
         # Check if key exists in DataTree by attempting to access it
@@ -234,7 +234,7 @@ def setup_datatree_metadata_geozarr_spec_compliant(
 
             # Set CRS if available
             if "proj:epsg" in ds[var_name].attrs:
-                epsg = ds[var_name].attrs["proj:epsg"]
+                epsg = make_epsg_code(ds[var_name].attrs["proj:epsg"])
                 log.info("Setting CRS for variable %s to EPSG %s", var_name, epsg)
                 ds = ds.rio.write_crs(f"epsg:{epsg}")
             elif epsg_CPM_260:

src/eopf_geozarr/conversion/s1_ingest.py

@@ -359,7 +359,7 @@ def _create_spatial_coordinate_arrays(
 # zarr-direct, so the attribute must be set explicitly.
 FLOAT32_NAN_FILL_VALUE = FillValueCoder.encode(np.nan, np.dtype("float32"))
 # CF metadata for the backscatter bands (vv/vh).
-BACKSCATTER_CF_ATTRS: S1BackscatterAttrsJSON = {
+BACKSCATTER_CF_ATTRS: Final[S1BackscatterAttrsJSON] = {
     "standard_name": "surface_backwards_scattering_coefficient_of_radar_wave",
     "units": "1",
     "_FillValue": FLOAT32_NAN_FILL_VALUE,

src/eopf_geozarr/s2_optimization/s2_converter.py

@@ -17,6 +17,12 @@
 from eopf_geozarr.conversion.geozarr import get_zarr_group
 from eopf_geozarr.data_api.s1 import Sentinel1Root
 from eopf_geozarr.data_api.s2 import Sentinel2Root
+from eopf_geozarr.types import (
+    BoundingBox2D,
+    make_bounding_box,
+    make_crs_code,
+    make_epsg_code,
+)
 
 from .s2_multiscale import create_multiscale_from_datatree
 
@@ -43,13 +49,8 @@ def initialize_crs_from_dataset(dt_input: xr.DataTree) -> CRS | None:
     )
     if epsg_cpm_260 is not None:
         try:
-            # Handle both integer (32632) and string ("EPSG:32632" or "32632") formats
-            if isinstance(epsg_cpm_260, str):
-                # Extract numeric part from string like "EPSG:32632" or "32632"
-                epsg_code = int(epsg_cpm_260.split(":")[-1])
-            else:
-                # Already an integer
-                epsg_code = int(epsg_cpm_260)
+            # Handles both integer (32632) and string ("EPSG:32632" or "32632") formats
+            epsg_code = make_epsg_code(epsg_cpm_260)
             crs = CRS.from_epsg(epsg_code)
             log.info("Initialized CRS from CPM 2.6.0+ metadata", epsg=epsg_code)
         except Exception as e:
@@ -97,7 +98,7 @@ def initialize_crs_from_dataset(dt_input: xr.DataTree) -> CRS | None:
             # Check for proj:epsg attribute
             if "proj:epsg" in var.attrs:
                 try:
-                    epsg = var.attrs["proj:epsg"]
+                    epsg = make_epsg_code(var.attrs["proj:epsg"])
                     crs = CRS.from_epsg(epsg)
                     log.info("Initialized CRS from EPSG code", epsg=epsg)
                 except Exception:
@@ -325,17 +326,17 @@ def simple_root_consolidation(output_path: str, datasets: Mapping[str, object])
     zarr.consolidate_metadata(output_path, zarr_format=3)
 
 
-def _as_bbox(value: object) -> tuple[float, float, float, float] | None:
-    """Return *value* as a 4-tuple of floats, or ``None`` if it is not one.
+def _as_bbox(value: object) -> BoundingBox2D | None:
+    """Return *value* as a validated bounding box, or ``None`` if it is not one.
 
     ``spatial:bbox`` is read from stored metadata, so its type is not known
-    statically; this verifies the shape at runtime rather than asserting it.
+    statically; unlike a bare ``make_bounding_box`` call, the store walker
+    tolerates absent or malformed values by skipping the group.
     """
-    if not isinstance(value, (list, tuple)) or len(value) != 4:
-        return None
-    if not all(isinstance(v, (int, float)) for v in value):
+    try:
+        return make_bounding_box(value)
+    except (TypeError, ValueError):
         return None
-    return (float(value[0]), float(value[1]), float(value[2]), float(value[3]))
 
 
 def write_store_root_bbox(output_path: str) -> None:
@@ -350,24 +351,27 @@ def write_store_root_bbox(output_path: str) -> None:
 
     root = zarr.open_group(output_path, mode="r+")
 
-    bboxes_4326: list[tuple[float, float, float, float]] = []
+    bboxes_4326: list[BoundingBox2D] = []
 
     def _walk(group: zarr.Group) -> None:
         attrs = dict(group.attrs)
         bbox = attrs.get("spatial:bbox")
-        code = attrs.get("proj:code")
+        raw_code = attrs.get("proj:code")
         # spatial:bbox comes from stored metadata; verify it is a 4-element
         # numeric sequence before use rather than trusting the type.
         corners = _as_bbox(bbox)
         if corners is not None:
             x0, y0, x1, y1 = corners
-            if code and code != "EPSG:4326":
+            # A group with a bbox but no proj:code is treated as already EPSG:4326;
+            # a present-but-malformed code fails loudly rather than being skipped.
+            code = make_crs_code(raw_code) if raw_code is not None else None
+            if code is not None and code != "EPSG:4326":
                 transformer = Transformer.from_crs(code, "EPSG:4326", always_xy=True)
                 xmin, ymin = transformer.transform(x0, y0)
                 xmax, ymax = transformer.transform(x1, y1)
-                bboxes_4326.append((xmin, ymin, xmax, ymax))
+                bboxes_4326.append(BoundingBox2D((xmin, ymin, xmax, ymax)))
             else:
-                bboxes_4326.append((x0, y0, x1, y1))
+                bboxes_4326.append(corners)
         for child in group.groups():
             _walk(child[1])
 

src/eopf_geozarr/s2_optimization/s2_multiscale.py

@@ -31,6 +31,7 @@
 )
 from eopf_geozarr.s2_optimization.common import DISTRIBUTED_AVAILABLE
 from eopf_geozarr.s2_optimization.s2_band_mapping import BAND_INFO
+from eopf_geozarr.types import make_epsg_code
 
 from .s2_resampling import determine_variable_type, downsample_variable
 
@@ -1150,7 +1151,7 @@ def write_geo_metadata(
                 crs = var.rio.crs
                 break
             if "proj:epsg" in var.attrs:
-                epsg = var.attrs["proj:epsg"]
+                epsg = make_epsg_code(var.attrs["proj:epsg"])
                 crs = CRS.from_epsg(epsg)
                 break
 

src/eopf_geozarr/types.py

@@ -21,6 +21,27 @@ def make_crs_code(value: object) -> CRSCode:
     return CRSCode(value)
 
 
+EPSGCode = NewType("EPSGCode", int)
+"""A bare EPSG integer code, e.g. ``32631`` (the legacy ``proj:epsg`` attribute value)."""
+
+
+def make_epsg_code(value: object) -> EPSGCode:
+    """Validate an untyped value (e.g. a zarr attribute) as an EPSG integer code.
+
+    Accepts an int (``32631``) or the string forms found in stored attrs and CPM
+    metadata (``"32631"``, ``"EPSG:32631"``). Validation is intentionally light:
+    pyproj's ``CRS.from_epsg`` raises ``CRSError`` downstream on codes it does
+    not recognize.
+    """
+    if isinstance(value, int) and not isinstance(value, bool):
+        return EPSGCode(value)
+    if isinstance(value, str):
+        tail = value.strip().split(":")[-1]
+        if tail.isdigit():
+            return EPSGCode(int(tail))
+    raise TypeError(f"EPSG code must be an int or 'EPSG:<int>' string, got {value!r}")
+
+
 def make_bounding_box(value: object) -> BoundingBox2D:
     """Validate an untyped value (e.g. a zarr attribute) as a 4-number bounding box.
 

tests/test_s2_multiscale_geo_metadata.py

@@ -288,10 +288,8 @@ def test_write_geo_metadata_invalid_crs(
         # Add invalid EPSG code
         ds["test_var"].attrs["proj:epsg"] = "invalid_epsg"
 
-        # Method should raise an exception for invalid CRS (normal behavior)
-        from pyproj.exceptions import CRSError
-
-        with pytest.raises(CRSError):
+        # make_epsg_code rejects the corrupt attr before it reaches pyproj
+        with pytest.raises(TypeError, match="EPSG code"):
             write_geo_metadata(ds)
 
     def test_write_geo_metadata_mixed_crs_variables(

tests/test_types.py

@@ -8,7 +8,7 @@
 
 import pytest
 
-from eopf_geozarr.types import make_bounding_box, make_crs_code
+from eopf_geozarr.types import make_bounding_box, make_crs_code, make_epsg_code
 
 # =============================================================================
 # make_crs_code
@@ -31,6 +31,29 @@ def test_crs_code_rejects_empty_or_whitespace(bad: str) -> None:
         make_crs_code(bad)
 
 
+# =============================================================================
+# make_epsg_code
+# =============================================================================
+
+
+def test_epsg_code_accepts_int() -> None:
+    assert make_epsg_code(32631) == 32631
+
+
+@pytest.mark.parametrize("value", ["32631", "EPSG:32631", "  EPSG:32631  "])
+def test_epsg_code_accepts_string_forms(value: str) -> None:
+    # Stored attrs and CPM metadata carry EPSG codes as "32631" or "EPSG:32631".
+    code = make_epsg_code(value)
+    assert code == 32631
+    assert isinstance(code, int)
+
+
+@pytest.mark.parametrize("bad", [None, True, 32631.0, ["EPSG:32631"], "EPSG:", "EPSG:abc", ""])
+def test_epsg_code_rejects_non_codes(bad: object) -> None:
+    with pytest.raises(TypeError, match="EPSG code"):
+        make_epsg_code(bad)
+
+
 # =============================================================================
 # make_bounding_box
 # =============================================================================