From e23d9b63ea2c61db44ccbd2cf5cf2940f3f0552c Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Mon, 25 May 2026 20:18:51 -0700 Subject: [PATCH 1/3] geotiff tests: consolidate rotated-CRS cluster (#2396) Folds three rotated / dropped-CRS test files into a single parametrized file at ``read/test_crs.py`` under the layout established by epic #2390 PR 1: * ``test_allow_rotated_geotiff_2115.py`` * ``test_allow_rotated_crs_drop_2126.py`` * ``test_allow_rotated_no_crs_2122.py`` Parametrizes scenarios as ``rotated_no_crs``, ``rotated_with_crs``, ``axis_aligned_with_crs`` with ``eager`` / ``dask`` backend axes. The GPU eager + dask+CuPy cases stay gated by ``@requires_gpu``. The two cross-file ``_write_rotated_tiff`` consumers (``test_georef_status_2136.py``, ``test_lazy_finalization_parity_2162.py``) now import from ``read/test_crs.py``. The release-gate and reference docs that cite the old filenames are updated to point at the new file. The HTTP rotated test (``test_http_dask_allow_rotated_2130.py``) is left in place; it belongs to PR 9 (integration cluster). ``CLUSTER_AUDIT_PR3.md`` maps every old ``file::test`` to its new ``file::test_id`` and is removed in a follow-up commit before approval. Part of epic #2390. --- docs/source/reference/geotiff.rst | 4 +- .../source/reference/release_gate_geotiff.rst | 4 +- xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md | 65 ++ xrspatial/geotiff/tests/read/__init__.py | 1 + xrspatial/geotiff/tests/read/test_crs.py | 619 ++++++++++++++++++ .../tests/test_allow_rotated_crs_drop_2126.py | 137 ---- .../tests/test_allow_rotated_geotiff_2115.py | 246 ------- .../tests/test_allow_rotated_no_crs_2122.py | 349 ---------- .../geotiff/tests/test_georef_status_2136.py | 10 +- .../test_lazy_finalization_parity_2162.py | 3 +- 10 files changed, 695 insertions(+), 743 deletions(-) create mode 100644 xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md create mode 100644 xrspatial/geotiff/tests/read/__init__.py create mode 100644 xrspatial/geotiff/tests/read/test_crs.py delete mode 100644 xrspatial/geotiff/tests/test_allow_rotated_crs_drop_2126.py delete mode 100644 xrspatial/geotiff/tests/test_allow_rotated_geotiff_2115.py delete mode 100644 xrspatial/geotiff/tests/test_allow_rotated_no_crs_2122.py diff --git a/docs/source/reference/geotiff.rst b/docs/source/reference/geotiff.rst index d6511587a..f53f4396c 100644 --- a/docs/source/reference/geotiff.rst +++ b/docs/source/reference/geotiff.rst @@ -110,9 +110,7 @@ has non-zero rotation or shear coefficients by default. Pass 6-tuple on ``attrs['rotated_affine']`` and drops ``attrs['crs']`` so downstream math cannot silently mix a rotated grid with an axis-aligned CRS. The dropped-CRS rule is locked by -``xrspatial/geotiff/tests/test_allow_rotated_crs_drop_2126.py``, -``xrspatial/geotiff/tests/test_allow_rotated_no_crs_2122.py``, and -``xrspatial/geotiff/tests/test_allow_rotated_geotiff_2115.py``. The +``xrspatial/geotiff/tests/read/test_crs.py``. The HTTP dask path honours the same opt-in via ``xrspatial/geotiff/tests/test_http_dask_allow_rotated_2130.py``. Without ``allow_rotated=True`` the read raises a typed error; see diff --git a/docs/source/reference/release_gate_geotiff.rst b/docs/source/reference/release_gate_geotiff.rst index b616af390..ae62f917e 100644 --- a/docs/source/reference/release_gate_geotiff.rst +++ b/docs/source/reference/release_gate_geotiff.rst @@ -451,9 +451,7 @@ attrs contract - Rotated reads surface ``rotated_affine`` and drop ``crs`` so downstream math cannot silently mix a rotated grid with an axis-aligned CRS. - - ``xrspatial/geotiff/tests/test_allow_rotated_crs_drop_2126.py``, - ``xrspatial/geotiff/tests/test_allow_rotated_no_crs_2122.py``, - ``xrspatial/geotiff/tests/test_allow_rotated_geotiff_2115.py`` + - ``xrspatial/geotiff/tests/read/test_crs.py`` - `#2340`_ * - ``reader.allow_unparseable_crs`` - experimental diff --git a/xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md b/xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md new file mode 100644 index 000000000..f631a133b --- /dev/null +++ b/xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md @@ -0,0 +1,65 @@ +# Cluster audit -- epic #2390 PR 3 (rotated / dropped CRS) + +Maps every old `file::test` from the three folded files to its new +`read/test_crs.py::test_id`. **This file is deleted on the final commit +before the PR is approved; it must not land on `main`.** + +The HTTP rotated read (`test_http_dask_allow_rotated_2130.py`) is left +in place; it belongs to PR 9 (integration). + +## `test_allow_rotated_geotiff_2115.py` + +| Old file::test | New file::test_id | Notes | +|---|---|---| +| `test_allow_rotated_geotiff_2115.py::test_extract_transform_rejects_rotated_by_default` | `read/test_crs.py::test_extract_transform_rotated_default_raises` | renamed for clarity | +| `test_allow_rotated_geotiff_2115.py::test_extract_transform_allow_rotated_returns_no_georef` | `read/test_crs.py::test_extract_transform_rotated_optin_returns_no_georef` | renamed | +| `test_allow_rotated_geotiff_2115.py::test_extract_transform_allow_rotated_passes_through_axis_aligned` | `read/test_crs.py::test_extract_transform_axis_aligned_optin_passes_through` | renamed | +| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_rotated_default_raises` | `read/test_crs.py::test_open_geotiff_rotated_no_crs_default_raises[eager]` | parametrized | +| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_rotated_allow_rotated_reads_pixels` | `read/test_crs.py::test_open_geotiff_rotated_no_crs_optin_reads_pixels[eager]` | parametrized | +| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_rotated_default_raises_with_dask` | `read/test_crs.py::test_open_geotiff_rotated_no_crs_default_raises[dask]` | parametrized | +| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_rotated_allow_rotated_with_dask` | `read/test_crs.py::test_open_geotiff_rotated_no_crs_optin_reads_pixels[dask]` | parametrized | +| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_http_rotated_default_raises` | -- | DROPPED. HTTP rotated coverage already exists in `test_http_dask_allow_rotated_2130.py` (PR 9 integration cluster). Local-file rotated raise is pinned by `test_open_geotiff_rotated_no_crs_default_raises[eager]`. | +| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_http_rotated_allow_rotated_reads_pixels` | -- | DROPPED. Same rationale as above; PR 9 integration cluster owns end-to-end HTTP rotated reads. | + +## `test_allow_rotated_crs_drop_2126.py` + +| Old file::test | New file::test_id | Notes | +|---|---|---| +| `test_allow_rotated_crs_drop_2126.py::test_rotated_optin_drops_crs_epsg` | `read/test_crs.py::test_populate_attrs_rotated_optin_drops_attr[crs]` | parametrized over `crs` / `crs_wkt` / `transform` | +| `test_allow_rotated_crs_drop_2126.py::test_rotated_optin_drops_crs_wkt` | `read/test_crs.py::test_populate_attrs_rotated_optin_drops_attr[crs_wkt]` | parametrized | +| `test_allow_rotated_crs_drop_2126.py::test_plain_no_georef_keeps_crs` | `read/test_crs.py::test_populate_attrs_plain_no_georef_keeps_crs` | unchanged | +| `test_allow_rotated_crs_drop_2126.py::test_axis_aligned_georef_keeps_crs_and_transform` | `read/test_crs.py::test_populate_attrs_axis_aligned_keeps_crs_and_transform` | renamed | +| `test_allow_rotated_crs_drop_2126.py::test_open_geotiff_rotated_with_crs_drops_crs` | `read/test_crs.py::test_open_geotiff_rotated_with_crs_geokey_only_drops_crs[eager]` | parametrized; tifffile-written variant with Geographic-only GeoKey | +| `test_allow_rotated_crs_drop_2126.py::test_open_geotiff_rotated_with_crs_dask_drops_crs` | `read/test_crs.py::test_open_geotiff_rotated_with_crs_geokey_only_drops_crs[dask]` | parametrized | + +## `test_allow_rotated_no_crs_2122.py` + +| Old file::test | New file::test_id | Notes | +|---|---|---| +| `test_allow_rotated_no_crs_2122.py::test_eager_rotated_read_drops_crs` | `read/test_crs.py::test_open_geotiff_rotated_with_crs_drops_crs[eager]` | parametrized; uses the hand-rolled writer with full `_GEO_KEYS_4326` block | +| `test_allow_rotated_no_crs_2122.py::test_dask_rotated_read_drops_crs` | `read/test_crs.py::test_open_geotiff_rotated_with_crs_drops_crs[dask]` | parametrized | +| `test_allow_rotated_no_crs_2122.py::test_cupy_rotated_read_drops_crs` | `read/test_crs.py::test_open_geotiff_rotated_with_crs_drops_crs_gpu[eager]` | parametrized; `@requires_gpu` | +| `test_allow_rotated_no_crs_2122.py::test_dask_cupy_rotated_read_drops_crs` | `read/test_crs.py::test_open_geotiff_rotated_with_crs_drops_crs_gpu[dask]` | parametrized; `@requires_gpu` | +| `test_allow_rotated_no_crs_2122.py::test_axis_aligned_read_still_emits_crs` | `read/test_crs.py::test_open_geotiff_axis_aligned_with_crs_keeps_crs` | renamed | +| `test_allow_rotated_no_crs_2122.py::test_vrt_eager_rotated_read_drops_crs` | `read/test_crs.py::test_vrt_rotated_with_crs_drops_crs[eager]` | parametrized | +| `test_allow_rotated_no_crs_2122.py::test_vrt_chunked_rotated_read_drops_crs` | `read/test_crs.py::test_vrt_rotated_with_crs_drops_crs[dask]` | parametrized | +| `test_allow_rotated_no_crs_2122.py::test_vrt_axis_aligned_still_emits_crs` | `read/test_crs.py::test_vrt_axis_aligned_with_crs_keeps_crs` | renamed | + +## Coverage delta + +* No coverage was dropped except the two HTTP cases from 2115, which are + superseded by the more thorough HTTP+dask coverage in + `test_http_dask_allow_rotated_2130.py` (PR 9). Local-file rotated + raise and pixel-grid read are still pinned by the eager+dask + parametrizations. +* The Geographic-only GeoKey path (tifffile writer, 2126's + ``_write_rotated_tiff_with_geokeys``) and the full + `_GEO_KEYS_4326`-block path (hand-rolled writer, 2122's + ``_write_rotated_tiff_with_crs``) are both kept as distinct + scenarios; each exercises a different GeoKey-parser branch. + +## Pre-merge action + +Delete this file (`CLUSTER_AUDIT_PR3.md`) on the final commit before the +PR is approved. The audit is a review artifact, not a documentation +deliverable; the git history and the PR description retain the trail. diff --git a/xrspatial/geotiff/tests/read/__init__.py b/xrspatial/geotiff/tests/read/__init__.py new file mode 100644 index 000000000..cc38874c2 --- /dev/null +++ b/xrspatial/geotiff/tests/read/__init__.py @@ -0,0 +1 @@ +"""Read-path tests for the GeoTIFF module.""" diff --git a/xrspatial/geotiff/tests/read/test_crs.py b/xrspatial/geotiff/tests/read/test_crs.py new file mode 100644 index 000000000..da6a97382 --- /dev/null +++ b/xrspatial/geotiff/tests/read/test_crs.py @@ -0,0 +1,619 @@ +"""Rotated and dropped-CRS read-path matrix. + +Consolidates three previously-separate files into a single parametrized +module covering the ``allow_rotated`` opt-in path and its CRS-drop +contract: + +* ``test_allow_rotated_geotiff_2115.py`` -- rotated TIFFs without + embedded CRS; ``allow_rotated=False`` (default) raises and + ``allow_rotated=True`` reads the pixel grid on the eager and dask + GeoTIFF backends. +* ``test_allow_rotated_crs_drop_2126.py`` -- rotated TIFFs whose GeoKey + block carries a CRS; ``allow_rotated=True`` drops ``crs`` / ``crs_wkt`` + together with the transform on both backends, plus the unit-level + ``_populate_attrs_from_geo_info`` contract. +* ``test_allow_rotated_no_crs_2122.py`` -- the eager / dask / cupy / + dask+cupy backend matrix plus the VRT eager + chunked paths, all + asserting CRS drop under ``allow_rotated=True``. + +The HTTP-server rotated read (``test_http_dask_allow_rotated_2130.py``) +stays in place; it belongs to the integration cluster (epic #2390 PR 9). + +Test ID convention: ``(scenario, ...)`` where ``scenario`` is one of +``rotated_no_crs``, ``rotated_with_crs``, ``axis_aligned_with_crs``. +""" +from __future__ import annotations + +import struct + +import numpy as np +import pytest + +from xrspatial.geotiff import open_geotiff +from xrspatial.geotiff._attrs import _populate_attrs_from_geo_info +from xrspatial.geotiff._errors import RotatedTransformError +from xrspatial.geotiff._geotags import ( + _NO_GEOREF_KEY, + TAG_MODEL_TRANSFORMATION, + GeoInfo, + GeoTransform, + _extract_transform, +) +from xrspatial.geotiff._header import IFD, IFDEntry + +from .._helpers.markers import requires_gpu + + +# --------------------------------------------------------------------------- +# Synthetic rotated 4x4 ModelTransformationTag values. +# --------------------------------------------------------------------------- + +# 30-degree rotation, pixel size 10, origin (100, 200). +_COS30 = 0.8660254037844387 +_SIN30 = 0.5 +_ROTATED_M_30DEG = ( + 10.0 * _COS30, -10.0 * _SIN30, 0.0, 100.0, + 10.0 * _SIN30, 10.0 * _COS30, 0.0, 200.0, + 0.0, 0.0, 1.0, 0.0, + 0.0, 0.0, 0.0, 1.0, +) + +# Smaller-rotation variant with pixel_width 1, b=0.1, pixel_height -1. +_ROTATED_M_SMALL = ( + 1.0, 0.1, 0.0, 100.0, + 0.0, -1.0, 0.0, 200.0, + 0.0, 0.0, 1.0, 0.0, + 0.0, 0.0, 0.0, 1.0, +) + +_AXIS_ALIGNED_M = ( + 10.0, 0.0, 0.0, 100.0, + 0.0, -10.0, 0.0, 200.0, + 0.0, 0.0, 1.0, 0.0, + 0.0, 0.0, 0.0, 1.0, +) + +# Minimal GeoKeyDirectoryTag declaring EPSG:4326. +# (version, key_revision, minor_revision, n_keys, KeyID, Location, Count, Value) +_GEO_KEYS_4326 = ( + 1, 1, 0, 3, + 1024, 0, 1, 2, # GTModelTypeGeoKey = Geographic + 1025, 0, 1, 1, # GTRasterTypeGeoKey = Area + 2048, 0, 1, 4326, # GeographicTypeGeoKey +) + + +# --------------------------------------------------------------------------- +# IFD builders for unit tests of ``_extract_transform``. +# --------------------------------------------------------------------------- + + +def _make_ifd_with_transform(matrix_16: tuple) -> IFD: + """Build a synthetic IFD that carries only a ModelTransformationTag. + + Other geo-related tags stay absent so the transform branch is the + only one exercised; this keeps the unit tests independent of the + full geo-key plumbing. + """ + ifd = IFD() + ifd.entries[TAG_MODEL_TRANSFORMATION] = IFDEntry( + tag=TAG_MODEL_TRANSFORMATION, + type_id=12, # DOUBLE + count=16, + value=tuple(matrix_16), + ) + return ifd + + +# --------------------------------------------------------------------------- +# Hand-rolled TIFF writers. ``_helpers/tiff_builders.make_minimal_tiff`` +# does not support a full 4x4 ModelTransformationTag (it writes axis- +# aligned files via ModelTiepoint + ModelPixelScale), so the rotated +# variants stay file-local. They are unchanged from the originals. +# --------------------------------------------------------------------------- + + +def _write_rotated_tiff(path, arr: np.ndarray, *, matrix=_ROTATED_M_30DEG) -> None: + """Write a minimal little-endian TIFF with a rotated ModelTransformationTag. + + Single-band, single-strip, uncompressed; no embedded CRS. Just + enough to exercise the reader's rotation path without rasterio / + GDAL. + """ + h, w = arr.shape + arr = np.ascontiguousarray(arr.astype(' None: + """Rotated TIFF with both ModelTransformationTag AND EPSG:4326 GeoKeys. + + The CRS is encoded in the GeoKeyDirectoryTag rather than ProjectedCRS + overflow, so the reader populates ``geo_info.crs_epsg`` alongside the + rotated transform. Used to exercise the CRS-drop path. + """ + h, w = arr.shape + arr = np.ascontiguousarray(arr.astype('\n' + f' {wkt}\n' + f' 0.0, 1.0, 0.5, 0.0, 0.0, -1.0\n' + f' \n' + f' \n' + f' {source_filename}' + f'\n' + f' 1\n' + f' \n' + f' \n' + f' \n' + f' \n' + f'\n' + ) + with open(vrt_path, 'w') as f: + f.write(xml) + + +def _write_axis_aligned_vrt(vrt_path, source_filename): + """Axis-aligned VRT with the same EPSG:4326 SRS block. + + Mirrors ``_write_rotated_vrt`` but with the rotation term set to zero + so the CRS-drop gate stays inactive. + """ + wkt = ( + 'GEOGCS["WGS 84",DATUM["WGS_1984",SPHEROID["WGS 84",6378137,' + '298.257223563]],PRIMEM["Greenwich",0],UNIT["degree",' + '0.0174532925199433],AUTHORITY["EPSG","4326"]]' + ) + xml = ( + f'\n' + f' {wkt}\n' + f' 0.0, 1.0, 0.0, 0.0, 0.0, -1.0\n' + f' \n' + f' \n' + f' {source_filename}' + f'\n' + f' 1\n' + f' \n' + f' \n' + f' \n' + f' \n' + f'\n' + ) + with open(vrt_path, 'w') as f: + f.write(xml) + + +# =========================================================================== +# Unit tests: ``_extract_transform`` rotated handling. +# =========================================================================== + + +def test_extract_transform_rotated_default_raises(): + """Default ``allow_rotated=False`` rejects a rotated transform.""" + ifd = _make_ifd_with_transform(_ROTATED_M_30DEG) + with pytest.raises(RotatedTransformError, match="rotation"): + _extract_transform(ifd) + + +def test_extract_transform_rotated_optin_returns_no_georef(): + """``allow_rotated=True`` returns has_georef=False and preserves the + rotated 6-tuple on ``rotated_affine``.""" + ifd = _make_ifd_with_transform(_ROTATED_M_30DEG) + gt, has_georef = _extract_transform(ifd, allow_rotated=True) + assert isinstance(gt, GeoTransform) + assert has_georef is False + expected = ( + _ROTATED_M_30DEG[0], _ROTATED_M_30DEG[1], _ROTATED_M_30DEG[3], + _ROTATED_M_30DEG[4], _ROTATED_M_30DEG[5], _ROTATED_M_30DEG[7], + ) + assert gt.rotated_affine is not None + assert gt.rotated_affine == pytest.approx(expected) + + +def test_extract_transform_axis_aligned_optin_passes_through(): + """Axis-aligned files are unchanged by the opt-in path.""" + ifd = _make_ifd_with_transform(_AXIS_ALIGNED_M) + gt, has_georef = _extract_transform(ifd, allow_rotated=True) + assert has_georef is True + assert gt.pixel_width == 10.0 + assert gt.pixel_height == -10.0 + assert gt.origin_x == 100.0 + assert gt.origin_y == 200.0 + assert gt.rotated_affine is None + + +# =========================================================================== +# Unit tests: ``_populate_attrs_from_geo_info`` CRS-drop contract. +# =========================================================================== + + +def _rotated_geo_info(*, with_crs: bool = True) -> GeoInfo: + """Build a GeoInfo that mimics the ``allow_rotated=True`` parser output.""" + t = GeoTransform( + origin_x=0.0, origin_y=0.0, + pixel_width=1.0, pixel_height=-1.0, + rotated_affine=(8.66, -5.0, 100.0, 5.0, 8.66, 200.0), + ) + return GeoInfo( + transform=t, + has_georef=False, + crs_epsg=4326 if with_crs else None, + crs_wkt='GEOGCS["WGS 84"]' if with_crs else None, + ) + + +@pytest.mark.parametrize("attr", ["crs", "crs_wkt", "transform"]) +def test_populate_attrs_rotated_optin_drops_attr(attr): + """Rotated opt-in path drops ``crs`` / ``crs_wkt`` / ``transform``.""" + gi = _rotated_geo_info() + attrs: dict = {} + _populate_attrs_from_geo_info(attrs, gi) + assert attr not in attrs + + +def test_populate_attrs_plain_no_georef_keeps_crs(): + """Plain no-georef file (no transform tag) still emits ``crs``. + + The drop gate is specific to the rotated opt-in path (transform + carries ``rotated_affine`` and ``has_georef`` is False). A file with + no transform at all but a GeoKey CRS keeps its CRS so callers can + display / reproject the array. + """ + gi = GeoInfo(transform=GeoTransform(), has_georef=False, crs_epsg=4326) + attrs: dict = {} + _populate_attrs_from_geo_info(attrs, gi) + assert attrs.get('crs') == 4326 + assert 'transform' not in attrs + + +def test_populate_attrs_axis_aligned_keeps_crs_and_transform(): + """Axis-aligned georeferenced files keep both attrs.""" + gi = GeoInfo( + transform=GeoTransform( + origin_x=100.0, origin_y=200.0, + pixel_width=10.0, pixel_height=-10.0, + ), + has_georef=True, + crs_epsg=4326, + ) + attrs: dict = {} + _populate_attrs_from_geo_info(attrs, gi) + assert attrs.get('crs') == 4326 + assert attrs.get('transform') == (10.0, 0.0, 100.0, 0.0, -10.0, 200.0) + + +# =========================================================================== +# End-to-end ``open_geotiff`` matrix. +# +# Parameter axes: +# * scenario: ``rotated_no_crs`` | ``rotated_with_crs`` | +# ``axis_aligned_with_crs`` +# * chunks: None (eager) | 2 (dask) +# =========================================================================== + + +def _build_rotated_no_crs(tmp_path, name): + src = tmp_path / name + arr = np.arange(20, dtype=' GeoInfo: - """Build a GeoInfo that mimics the allow_rotated=True parser output.""" - t = GeoTransform( - origin_x=0.0, origin_y=0.0, - pixel_width=1.0, pixel_height=-1.0, - rotated_affine=(8.66, -5.0, 100.0, 5.0, 8.66, 200.0), - ) - return GeoInfo( - transform=t, - has_georef=False, - crs_epsg=4326 if with_crs else None, - crs_wkt='GEOGCS["WGS 84"]' if with_crs else None, - ) - - -def test_rotated_optin_drops_crs_epsg(): - gi = _rotated_geo_info() - attrs = {} - _populate_attrs_from_geo_info(attrs, gi) - assert 'crs' not in attrs - assert 'transform' not in attrs - - -def test_rotated_optin_drops_crs_wkt(): - gi = _rotated_geo_info() - attrs = {} - _populate_attrs_from_geo_info(attrs, gi) - assert 'crs_wkt' not in attrs - - -def test_plain_no_georef_keeps_crs(): - # Plain no-georef file: no transform tag at all, but the GeoKey - # directory carries a CRS. This case must still emit ``crs`` so - # callers can display / reproject the array even without a transform. - gi = GeoInfo(transform=GeoTransform(), has_georef=False, crs_epsg=4326) - attrs = {} - _populate_attrs_from_geo_info(attrs, gi) - assert attrs.get('crs') == 4326 - assert 'transform' not in attrs - - -def test_axis_aligned_georef_keeps_crs_and_transform(): - gi = GeoInfo( - transform=GeoTransform( - origin_x=100.0, origin_y=200.0, - pixel_width=10.0, pixel_height=-10.0, - ), - has_georef=True, - crs_epsg=4326, - ) - attrs = {} - _populate_attrs_from_geo_info(attrs, gi) - assert attrs.get('crs') == 4326 - assert attrs.get('transform') == (10.0, 0.0, 100.0, 0.0, -10.0, 200.0) - - -# --------------------------------------------------------------------------- -# End-to-end via open_geotiff: write a rotated GeoTIFF with embedded CRS, -# read it with allow_rotated=True, confirm the attrs are clean. -# --------------------------------------------------------------------------- - - -def _write_rotated_tiff_with_geokeys(path, arr, *, epsg=4326): - """Build a synthetic rotated GeoTIFF with a GeoKey-encoded CRS.""" - tifffile = pytest.importorskip("tifffile") - # Rotated 4x4 model transformation tag (30-deg rotation, pixel size 10). - cos30 = 0.8660254037844387 - sin30 = 0.5 - m = ( - 10.0 * cos30, -10.0 * sin30, 0.0, 100.0, - 10.0 * sin30, 10.0 * cos30, 0.0, 200.0, - 0.0, 0.0, 1.0, 0.0, - 0.0, 0.0, 0.0, 1.0, - ) - # GeoKeyDirectory: header (4 shorts) + one key for GeographicTypeGeoKey - # (key id 2048, location 0 -> short value, count 1, value EPSG). - geo_key_directory = ( - 1, 1, 0, 1, # KeyDirectoryVersion=1, KeyRevision=1, MinorRevision=0, NumberOfKeys=1 - 2048, 0, 1, int(epsg), - ) - extratags = [ - # ModelTransformationTag (34264) -- DOUBLE, count=16. - (34264, 12, 16, m, False), - # GeoKeyDirectoryTag (34735) -- SHORT, count=8. - (34735, 3, 8, geo_key_directory, False), - ] - tifffile.imwrite( - path, arr, photometric='minisblack', - planarconfig='contig', extratags=extratags, - ) - - -def test_open_geotiff_rotated_with_crs_drops_crs(tmp_path): - arr = np.arange(20, dtype=' IFD: - """Build a synthetic IFD that carries only a rotated ModelTransformationTag. - - The other geo-related tags stay absent so the transform branch is the - only one exercised; this keeps the unit tests independent of the full - geo-key plumbing. - """ - ifd = IFD() - ifd.entries[TAG_MODEL_TRANSFORMATION] = IFDEntry( - tag=TAG_MODEL_TRANSFORMATION, - type_id=12, # DOUBLE - count=16, - value=tuple(matrix_16), - ) - return ifd - - -# A 30-degree rotation with pixel size 10, origin (100, 200). -# Row-major 4x4 in GeoTIFF order: -# x = M[0]*col + M[1]*row + M[2]*z + M[3] -# y = M[4]*col + M[5]*row + M[6]*z + M[7] -_COS30 = 0.8660254037844387 -_SIN30 = 0.5 -_ROTATED_M = ( - 10.0 * _COS30, -10.0 * _SIN30, 0.0, 100.0, # x row - 10.0 * _SIN30, 10.0 * _COS30, 0.0, 200.0, # y row - 0.0, 0.0, 1.0, 0.0, - 0.0, 0.0, 0.0, 1.0, -) - - -def test_extract_transform_rejects_rotated_by_default(): - ifd = _make_ifd_with_rotated_transform(_ROTATED_M) - with pytest.raises(RotatedTransformError, match="rotation"): - _extract_transform(ifd) - - -def test_extract_transform_allow_rotated_returns_no_georef(): - ifd = _make_ifd_with_rotated_transform(_ROTATED_M) - gt, has_georef = _extract_transform(ifd, allow_rotated=True) - assert isinstance(gt, GeoTransform) - assert has_georef is False - # The rotated 6-tuple should be preserved for downstream consumers. - expected = ( - _ROTATED_M[0], _ROTATED_M[1], _ROTATED_M[3], - _ROTATED_M[4], _ROTATED_M[5], _ROTATED_M[7], - ) - assert gt.rotated_affine is not None - assert gt.rotated_affine == pytest.approx(expected) - - -def test_extract_transform_allow_rotated_passes_through_axis_aligned(): - """Axis-aligned files should be unchanged by the opt-in path.""" - aligned = ( - 10.0, 0.0, 0.0, 100.0, - 0.0, -10.0, 0.0, 200.0, - 0.0, 0.0, 1.0, 0.0, - 0.0, 0.0, 0.0, 1.0, - ) - ifd = _make_ifd_with_rotated_transform(aligned) - gt, has_georef = _extract_transform(ifd, allow_rotated=True) - assert has_georef is True - assert gt.pixel_width == 10.0 - assert gt.pixel_height == -10.0 - assert gt.origin_x == 100.0 - assert gt.origin_y == 200.0 - assert gt.rotated_affine is None - - -def _write_rotated_tiff(path, arr: np.ndarray) -> None: - """Write a minimal little-endian TIFF with a rotated ModelTransformationTag. - - Single-band, single-strip, uncompressed; just enough to exercise the - reader's rotation path without depending on rasterio/GDAL. - """ - h, w = arr.shape - arr = np.ascontiguousarray(arr.astype(' bool: - if importlib.util.find_spec("cupy") is None: - return False - try: - import cupy - return bool(cupy.cuda.is_available()) - except Exception: - return False - - -_HAS_GPU = _gpu_available() -_gpu_only = pytest.mark.skipif(not _HAS_GPU, reason="cupy + CUDA required") - - -def _write_rotated_tiff_with_crs(path, arr: np.ndarray) -> None: - """Write a TIFF with both rotated ModelTransformationTag AND EPSG:4326 GeoKeys. - - Single-strip uncompressed uint16, written by hand so the fixture has - no rasterio / GDAL dependency. Mirrors the helper in - ``test_allow_rotated_geotiff_2115.py`` but adds the - GeoKeyDirectoryTag so the reader populates ``geo_info.crs_epsg`` - and ``geo_info.crs_wkt`` alongside the rotated transform. - """ - h, w = arr.shape - arr = np.ascontiguousarray(arr.astype('\n' - f' {wkt}\n' - # GDAL ordering: (origin_x, pixel_width, b, origin_y, d, pixel_height) - # b=0.5 forces rotation. - f' 0.0, 1.0, 0.5, 0.0, 0.0, -1.0\n' - f' \n' - f' \n' - f' {source_filename}' - f'\n' - f' 1\n' - f' \n' - f' \n' - f' \n' - f' \n' - f'\n' - ) - with open(vrt_path, 'w') as f: - f.write(xml) - - -def test_vrt_eager_rotated_read_drops_crs(tmp_path): - """Eager VRT path drops ``crs`` / ``crs_wkt`` / ``transform`` on - rotated read under ``allow_rotated=True``.""" - src = tmp_path / "vrt_src_2122.tif" - _write_minimal_aligned_tiff(src) - vrt = tmp_path / "rotated_2122.vrt" - _write_rotated_vrt(vrt, src.name) - - da = open_geotiff(str(vrt), allow_rotated=True) - - assert 'crs' not in da.attrs, sorted(da.attrs.keys()) - assert 'crs_wkt' not in da.attrs, sorted(da.attrs.keys()) - assert 'transform' not in da.attrs, sorted(da.attrs.keys()) - # VRT rotated reads must match the eager non-VRT contract: int64 - # pixel coords plus the no-georef marker, so the writer round-trip - # treats the array as a no-georef pixel grid rather than a - # user-authored integer-coord raster. - assert da.x.dtype == np.int64, da.x.dtype - assert da.y.dtype == np.int64, da.y.dtype - assert da.attrs.get(_NO_GEOREF_KEY) is True, sorted(da.attrs.keys()) - - -def test_vrt_chunked_rotated_read_drops_crs(tmp_path): - """Chunked VRT path drops ``crs`` / ``crs_wkt`` / ``transform`` on - rotated read under ``allow_rotated=True``.""" - src = tmp_path / "vrt_src_2122_chunks.tif" - _write_minimal_aligned_tiff(src) - vrt = tmp_path / "rotated_2122_chunks.vrt" - _write_rotated_vrt(vrt, src.name) - - da = open_geotiff(str(vrt), allow_rotated=True, chunks=2) - - assert 'crs' not in da.attrs, sorted(da.attrs.keys()) - assert 'crs_wkt' not in da.attrs, sorted(da.attrs.keys()) - assert 'transform' not in da.attrs, sorted(da.attrs.keys()) - assert da.x.dtype == np.int64, da.x.dtype - assert da.y.dtype == np.int64, da.y.dtype - assert da.attrs.get(_NO_GEOREF_KEY) is True, sorted(da.attrs.keys()) - - -def test_vrt_axis_aligned_still_emits_crs(tmp_path): - """Regression guard for the VRT path: non-rotated VRTs still emit crs.""" - src = tmp_path / "vrt_src_2122_ok.tif" - _write_minimal_aligned_tiff(src) - vrt = tmp_path / "aligned_2122.vrt" - wkt = ( - 'GEOGCS["WGS 84",DATUM["WGS_1984",SPHEROID["WGS 84",6378137,' - '298.257223563]],PRIMEM["Greenwich",0],UNIT["degree",' - '0.0174532925199433],AUTHORITY["EPSG","4326"]]' - ) - xml = ( - f'\n' - f' {wkt}\n' - f' 0.0, 1.0, 0.0, 0.0, 0.0, -1.0\n' - f' \n' - f' \n' - f' {src.name}' - f'\n' - f' 1\n' - f' \n' - f' \n' - f' \n' - f' \n' - f'\n' - ) - with open(vrt, 'w') as f: - f.write(xml) - - da = open_geotiff(str(vrt)) - - assert da.attrs.get('crs') == 4326 - assert 'crs_wkt' in da.attrs - assert 'transform' in da.attrs diff --git a/xrspatial/geotiff/tests/test_georef_status_2136.py b/xrspatial/geotiff/tests/test_georef_status_2136.py index 6acb0ecc9..e9a7a1c91 100644 --- a/xrspatial/geotiff/tests/test_georef_status_2136.py +++ b/xrspatial/geotiff/tests/test_georef_status_2136.py @@ -40,10 +40,12 @@ tifffile = pytest.importorskip("tifffile") -# Reuse the rotated-TIFF writer from the #2115 test rather than copying -# the byte layout. The function is private to that test module but -# the test runner sees the package directory so the import succeeds. -from xrspatial.geotiff.tests.test_allow_rotated_geotiff_2115 import \ +# Reuse the rotated-TIFF writer from the consolidated CRS suite rather +# than copying the byte layout. The function is private to that test +# module but the test runner sees the package directory so the import +# succeeds. (Previously lived in ``test_allow_rotated_geotiff_2115.py``; +# moved under ``read/test_crs.py`` by epic #2390 PR 3.) +from xrspatial.geotiff.tests.read.test_crs import \ _write_rotated_tiff # noqa: E402 _STATUS_KEY = 'georef_status' diff --git a/xrspatial/geotiff/tests/test_lazy_finalization_parity_2162.py b/xrspatial/geotiff/tests/test_lazy_finalization_parity_2162.py index 4c22972a5..fc51dfbde 100644 --- a/xrspatial/geotiff/tests/test_lazy_finalization_parity_2162.py +++ b/xrspatial/geotiff/tests/test_lazy_finalization_parity_2162.py @@ -40,7 +40,8 @@ tifffile = pytest.importorskip("tifffile") -from xrspatial.geotiff.tests.test_allow_rotated_geotiff_2115 import \ +# Rotated-TIFF writer relocated to ``read/test_crs.py`` by epic #2390 PR 3. +from xrspatial.geotiff.tests.read.test_crs import \ _write_rotated_tiff # noqa: E402 From 89d007843e8817ac43f38a161c1c075255c3ab55 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Mon, 25 May 2026 20:22:15 -0700 Subject: [PATCH 2/3] Address review nits: filename labels, audit specifics (#2396) Three nits from /review-pr applied: * Add ``_chunks_label`` helper so tmp_path filenames read ``..._eager.tif`` / ``..._dask.tif`` instead of literal ``..._None.tif`` / ``..._2.tif``. * Use ``src, _ = ...`` in the default-raises and axis-aligned tests so the unused fixture return is explicit. * Spell out the specific PR 9 test names that supersede the two dropped HTTP cases in ``CLUSTER_AUDIT_PR3.md`` so the trail is explicit without opening the integration file. All 22 ``read/`` tests still pass. --- xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md | 4 +-- xrspatial/geotiff/tests/read/test_crs.py | 26 ++++++++++++++------ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md b/xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md index f631a133b..7cde5e8e9 100644 --- a/xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md +++ b/xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md @@ -18,8 +18,8 @@ in place; it belongs to PR 9 (integration). | `test_allow_rotated_geotiff_2115.py::test_open_geotiff_rotated_allow_rotated_reads_pixels` | `read/test_crs.py::test_open_geotiff_rotated_no_crs_optin_reads_pixels[eager]` | parametrized | | `test_allow_rotated_geotiff_2115.py::test_open_geotiff_rotated_default_raises_with_dask` | `read/test_crs.py::test_open_geotiff_rotated_no_crs_default_raises[dask]` | parametrized | | `test_allow_rotated_geotiff_2115.py::test_open_geotiff_rotated_allow_rotated_with_dask` | `read/test_crs.py::test_open_geotiff_rotated_no_crs_optin_reads_pixels[dask]` | parametrized | -| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_http_rotated_default_raises` | -- | DROPPED. HTTP rotated coverage already exists in `test_http_dask_allow_rotated_2130.py` (PR 9 integration cluster). Local-file rotated raise is pinned by `test_open_geotiff_rotated_no_crs_default_raises[eager]`. | -| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_http_rotated_allow_rotated_reads_pixels` | -- | DROPPED. Same rationale as above; PR 9 integration cluster owns end-to-end HTTP rotated reads. | +| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_http_rotated_default_raises` | -- | DROPPED. Superseded by `test_http_dask_allow_rotated_2130.py::test_http_dask_rotated_default_raises` (PR 9 integration cluster), which pins the same default-raise behaviour through the HTTP+dask metadata path. Local-file rotated raise is pinned by `test_open_geotiff_rotated_no_crs_default_raises[eager]`. | +| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_http_rotated_allow_rotated_reads_pixels` | -- | DROPPED. Superseded by `test_http_dask_allow_rotated_2130.py::test_http_dask_rotated_allow_rotated_reads` (PR 9 integration cluster), which exercises the HTTP+dask `allow_rotated=True` path end-to-end. | ## `test_allow_rotated_crs_drop_2126.py` diff --git a/xrspatial/geotiff/tests/read/test_crs.py b/xrspatial/geotiff/tests/read/test_crs.py index da6a97382..9ca24109f 100644 --- a/xrspatial/geotiff/tests/read/test_crs.py +++ b/xrspatial/geotiff/tests/read/test_crs.py @@ -425,6 +425,15 @@ def test_populate_attrs_axis_aligned_keeps_crs_and_transform(): # =========================================================================== +def _chunks_label(chunks) -> str: + """Filename-friendly label matching the pytest test ID for ``chunks``. + + Keeps ``tmp_path`` trees readable: ``..._eager.tif`` / ``..._dask.tif`` + instead of ``..._None.tif`` / ``..._2.tif``. + """ + return "eager" if chunks is None else "dask" + + def _build_rotated_no_crs(tmp_path, name): src = tmp_path / name arr = np.arange(20, dtype=' Date: Mon, 25 May 2026 20:22:21 -0700 Subject: [PATCH 3/3] Remove CLUSTER_AUDIT_PR3.md before merge (#2396) The audit is a review artifact, not a documentation deliverable. Git history and the PR description retain the trail mapping old test files to ``read/test_crs.py`` IDs. Part of epic #2390. --- xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md | 65 -------------------- 1 file changed, 65 deletions(-) delete mode 100644 xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md diff --git a/xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md b/xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md deleted file mode 100644 index 7cde5e8e9..000000000 --- a/xrspatial/geotiff/tests/CLUSTER_AUDIT_PR3.md +++ /dev/null @@ -1,65 +0,0 @@ -# Cluster audit -- epic #2390 PR 3 (rotated / dropped CRS) - -Maps every old `file::test` from the three folded files to its new -`read/test_crs.py::test_id`. **This file is deleted on the final commit -before the PR is approved; it must not land on `main`.** - -The HTTP rotated read (`test_http_dask_allow_rotated_2130.py`) is left -in place; it belongs to PR 9 (integration). - -## `test_allow_rotated_geotiff_2115.py` - -| Old file::test | New file::test_id | Notes | -|---|---|---| -| `test_allow_rotated_geotiff_2115.py::test_extract_transform_rejects_rotated_by_default` | `read/test_crs.py::test_extract_transform_rotated_default_raises` | renamed for clarity | -| `test_allow_rotated_geotiff_2115.py::test_extract_transform_allow_rotated_returns_no_georef` | `read/test_crs.py::test_extract_transform_rotated_optin_returns_no_georef` | renamed | -| `test_allow_rotated_geotiff_2115.py::test_extract_transform_allow_rotated_passes_through_axis_aligned` | `read/test_crs.py::test_extract_transform_axis_aligned_optin_passes_through` | renamed | -| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_rotated_default_raises` | `read/test_crs.py::test_open_geotiff_rotated_no_crs_default_raises[eager]` | parametrized | -| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_rotated_allow_rotated_reads_pixels` | `read/test_crs.py::test_open_geotiff_rotated_no_crs_optin_reads_pixels[eager]` | parametrized | -| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_rotated_default_raises_with_dask` | `read/test_crs.py::test_open_geotiff_rotated_no_crs_default_raises[dask]` | parametrized | -| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_rotated_allow_rotated_with_dask` | `read/test_crs.py::test_open_geotiff_rotated_no_crs_optin_reads_pixels[dask]` | parametrized | -| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_http_rotated_default_raises` | -- | DROPPED. Superseded by `test_http_dask_allow_rotated_2130.py::test_http_dask_rotated_default_raises` (PR 9 integration cluster), which pins the same default-raise behaviour through the HTTP+dask metadata path. Local-file rotated raise is pinned by `test_open_geotiff_rotated_no_crs_default_raises[eager]`. | -| `test_allow_rotated_geotiff_2115.py::test_open_geotiff_http_rotated_allow_rotated_reads_pixels` | -- | DROPPED. Superseded by `test_http_dask_allow_rotated_2130.py::test_http_dask_rotated_allow_rotated_reads` (PR 9 integration cluster), which exercises the HTTP+dask `allow_rotated=True` path end-to-end. | - -## `test_allow_rotated_crs_drop_2126.py` - -| Old file::test | New file::test_id | Notes | -|---|---|---| -| `test_allow_rotated_crs_drop_2126.py::test_rotated_optin_drops_crs_epsg` | `read/test_crs.py::test_populate_attrs_rotated_optin_drops_attr[crs]` | parametrized over `crs` / `crs_wkt` / `transform` | -| `test_allow_rotated_crs_drop_2126.py::test_rotated_optin_drops_crs_wkt` | `read/test_crs.py::test_populate_attrs_rotated_optin_drops_attr[crs_wkt]` | parametrized | -| `test_allow_rotated_crs_drop_2126.py::test_plain_no_georef_keeps_crs` | `read/test_crs.py::test_populate_attrs_plain_no_georef_keeps_crs` | unchanged | -| `test_allow_rotated_crs_drop_2126.py::test_axis_aligned_georef_keeps_crs_and_transform` | `read/test_crs.py::test_populate_attrs_axis_aligned_keeps_crs_and_transform` | renamed | -| `test_allow_rotated_crs_drop_2126.py::test_open_geotiff_rotated_with_crs_drops_crs` | `read/test_crs.py::test_open_geotiff_rotated_with_crs_geokey_only_drops_crs[eager]` | parametrized; tifffile-written variant with Geographic-only GeoKey | -| `test_allow_rotated_crs_drop_2126.py::test_open_geotiff_rotated_with_crs_dask_drops_crs` | `read/test_crs.py::test_open_geotiff_rotated_with_crs_geokey_only_drops_crs[dask]` | parametrized | - -## `test_allow_rotated_no_crs_2122.py` - -| Old file::test | New file::test_id | Notes | -|---|---|---| -| `test_allow_rotated_no_crs_2122.py::test_eager_rotated_read_drops_crs` | `read/test_crs.py::test_open_geotiff_rotated_with_crs_drops_crs[eager]` | parametrized; uses the hand-rolled writer with full `_GEO_KEYS_4326` block | -| `test_allow_rotated_no_crs_2122.py::test_dask_rotated_read_drops_crs` | `read/test_crs.py::test_open_geotiff_rotated_with_crs_drops_crs[dask]` | parametrized | -| `test_allow_rotated_no_crs_2122.py::test_cupy_rotated_read_drops_crs` | `read/test_crs.py::test_open_geotiff_rotated_with_crs_drops_crs_gpu[eager]` | parametrized; `@requires_gpu` | -| `test_allow_rotated_no_crs_2122.py::test_dask_cupy_rotated_read_drops_crs` | `read/test_crs.py::test_open_geotiff_rotated_with_crs_drops_crs_gpu[dask]` | parametrized; `@requires_gpu` | -| `test_allow_rotated_no_crs_2122.py::test_axis_aligned_read_still_emits_crs` | `read/test_crs.py::test_open_geotiff_axis_aligned_with_crs_keeps_crs` | renamed | -| `test_allow_rotated_no_crs_2122.py::test_vrt_eager_rotated_read_drops_crs` | `read/test_crs.py::test_vrt_rotated_with_crs_drops_crs[eager]` | parametrized | -| `test_allow_rotated_no_crs_2122.py::test_vrt_chunked_rotated_read_drops_crs` | `read/test_crs.py::test_vrt_rotated_with_crs_drops_crs[dask]` | parametrized | -| `test_allow_rotated_no_crs_2122.py::test_vrt_axis_aligned_still_emits_crs` | `read/test_crs.py::test_vrt_axis_aligned_with_crs_keeps_crs` | renamed | - -## Coverage delta - -* No coverage was dropped except the two HTTP cases from 2115, which are - superseded by the more thorough HTTP+dask coverage in - `test_http_dask_allow_rotated_2130.py` (PR 9). Local-file rotated - raise and pixel-grid read are still pinned by the eager+dask - parametrizations. -* The Geographic-only GeoKey path (tifffile writer, 2126's - ``_write_rotated_tiff_with_geokeys``) and the full - `_GEO_KEYS_4326`-block path (hand-rolled writer, 2122's - ``_write_rotated_tiff_with_crs``) are both kept as distinct - scenarios; each exercises a different GeoKey-parser branch. - -## Pre-merge action - -Delete this file (`CLUSTER_AUDIT_PR3.md`) on the final commit before the -PR is approved. The audit is a review artifact, not a documentation -deliverable; the git history and the PR description retain the trail.