From 1a1ad8c7473e0db6c1b0e65aecac93a2de9ef488 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Wed, 20 May 2026 19:48:41 -0700 Subject: [PATCH 1/2] geotiff: refuse to silently drop attrs['rotated_affine'] on write (#2216) The reader exposes the rotated 6-tuple on attrs['rotated_affine'] when called with allow_rotated=True (issue #2129), but to_geotiff has no ModelTransformationTag emit path (tracked in #2115). Without an explicit gate the writer accepted such inputs and produced an identity-affine output, losing the rotation without any signal to the caller. Add a shared _validate_no_rotated_affine helper in _validation.py and call it from to_geotiff, write_geotiff_gpu, and _write_vrt_tiled. The default refuses the write with a ValueError that names the attr and points at the opt-in; a new drop_rotation=True kwarg lets the caller accept the loss explicitly. Docstrings on to_geotiff, write_geotiff_gpu, and the open_geotiff allow_rotated entry document the new gate. Tests in test_to_geotiff_drop_rotation_2216.py cover the rejection path, the opt-in path, the non-rotated baseline, a None-valued attr, and read-then-write-then-read round-trips on a synthetic rotated TIFF (eager and dask). VRT path parity is pinned via the to_geotiff(.vrt) dispatch and a direct _write_vrt_tiled call. --- xrspatial/geotiff/__init__.py | 16 +- xrspatial/geotiff/_geotags.py | 10 +- xrspatial/geotiff/_validation.py | 42 +++ xrspatial/geotiff/_writers/eager.py | 58 +++- xrspatial/geotiff/_writers/gpu.py | 22 +- .../test_to_geotiff_drop_rotation_2216.py | 270 ++++++++++++++++++ 6 files changed, 403 insertions(+), 15 deletions(-) create mode 100644 xrspatial/geotiff/tests/test_to_geotiff_drop_rotation_2216.py diff --git a/xrspatial/geotiff/__init__.py b/xrspatial/geotiff/__init__.py index 21e174b88..c12115d8a 100644 --- a/xrspatial/geotiff/__init__.py +++ b/xrspatial/geotiff/__init__.py @@ -446,9 +446,10 @@ def open_geotiff(source: str | BinaryIO, *, pixel, and ``dtype=`` then raises ``ValueError`` on the float-to-int cast. allow_rotated : bool, default False - Advanced: read-only opt-in; ``to_geotiff`` does not currently - emit ``rotated_affine`` so a read-then-write round-trip writes - an identity-affine output and silently drops the rotation. + Advanced: read-only opt-in. ``to_geotiff`` does not currently + emit ``rotated_affine``; it rejects DataArrays that carry the + attr (``ValueError`` naming the attr) unless the caller passes + ``drop_rotation=True`` to accept the loss explicitly (#2216). Read-side opt-in for rotated / sheared ``ModelTransformationTag`` files. By default the reader raises ``NotImplementedError`` because the rest of xrspatial assumes an axis-aligned grid. @@ -463,10 +464,11 @@ def open_geotiff(source: str | BinaryIO, *, ``attrs['rotated_affine']`` as ``(a, b, c, d, e, f)`` (rasterio ``Affine`` ordering) so consumers that know how to handle rotated rasters can recover the mapping (issue #2129). The - contract is read-only -- ``to_geotiff`` does not currently - emit rotated transforms, so a read-then-write round-trip - writes an identity-affine output and silently drops the - rotation (issue #2115). + contract is read-only -- writes must either reproject onto an + axis-aligned grid first, or pass ``drop_rotation=True`` to + ``to_geotiff`` / ``write_geotiff_gpu`` to accept the loss; the + ``ModelTransformationTag`` emit path is tracked separately + (issue #2115). Returns ------- diff --git a/xrspatial/geotiff/_geotags.py b/xrspatial/geotiff/_geotags.py index 3a4c7ebb3..b6a5effc9 100644 --- a/xrspatial/geotiff/_geotags.py +++ b/xrspatial/geotiff/_geotags.py @@ -633,10 +633,12 @@ def _extract_transform(ifd: IFD, raise ``NotImplementedError``. This contract is read-only. ``rotated_affine`` is not currently - emitted by the writer, so a read-with-``allow_rotated`` - followed by ``to_geotiff`` round-trip silently writes an - identity-affine output and drops the original rotation. If - round-trip preservation matters, the writer needs a separate + emitted by the writer. As of issue #2216 the writer refuses + such inputs with a ``ValueError`` naming the attr unless the + caller passes ``drop_rotation=True`` to accept the loss + explicitly; the silent identity-affine round-trip the previous + wording warned about is no longer reachable. If round-trip + preservation matters, the writer needs a separate ``ModelTransformationTag`` emit path that consumes ``rotated_affine`` (see issue #2115 follow-up). diff --git a/xrspatial/geotiff/_validation.py b/xrspatial/geotiff/_validation.py index 57406504b..0bcb35c0d 100644 --- a/xrspatial/geotiff/_validation.py +++ b/xrspatial/geotiff/_validation.py @@ -583,6 +583,48 @@ def _validate_nodata_arg(nodata) -> None: ) from e +def _validate_no_rotated_affine(attrs, *, drop_rotation: bool, + entry_point: str = "to_geotiff") -> None: + """Refuse writes that would silently drop ``attrs['rotated_affine']``. + + The reader exposes the rotated 6-tuple on ``attrs['rotated_affine']`` + when called with ``allow_rotated=True`` (issue #2129). The writer + does not emit a ``ModelTransformationTag`` (tracked in #2115), so a + read-then-write round-trip used to silently drop the rotation and + write an identity-affine output (issue #2216). Refuse the write by + default so the loss is impossible without an explicit opt-in. + + Parameters + ---------- + attrs : Mapping or None + The DataArray's ``attrs``. Bare ``numpy.ndarray`` / ``cupy`` + inputs have no attrs and skip the check. + drop_rotation : bool + When True, the caller has explicitly accepted that the rotated + mapping will be lost on write; the check returns silently. + entry_point : str + Name of the calling writer for the error message (``to_geotiff``, + ``write_geotiff_gpu``). Lets the two writers surface the same + wording while still naming the opt-in correctly for either entry + point. + """ + if not attrs: + return + if attrs.get('rotated_affine') is None: + return + if drop_rotation: + return + raise ValueError( + f"{entry_point}: refusing to write a DataArray carrying " + f"attrs['rotated_affine']. The writer does not emit a " + f"ModelTransformationTag (issue #2115), so writing this input " + f"would silently drop the rotated mapping and produce an " + f"identity-affine GeoTIFF. Either reproject onto an axis-aligned " + f"grid first, or pass drop_rotation=True to accept the loss " + f"explicitly (issue #2216)." + ) + + # --------------------------------------------------------------------------- # Ambiguous-metadata hooks (issue #1987 PR 0) # diff --git a/xrspatial/geotiff/_writers/eager.py b/xrspatial/geotiff/_writers/eager.py index b8fadb891..235b99a4f 100644 --- a/xrspatial/geotiff/_writers/eager.py +++ b/xrspatial/geotiff/_writers/eager.py @@ -46,6 +46,7 @@ ) from .._validation import ( _validate_3d_writer_dims, + _validate_no_rotated_affine, _validate_nodata_arg, _validate_tile_size_arg, _validate_writer_spatial_shape, @@ -74,7 +75,8 @@ def to_geotiff(data: xr.DataArray | np.ndarray, photometric: str | int = 'auto', allow_internal_only_jpeg: bool = False, allow_experimental_codecs: bool = False, - allow_unparseable_crs: bool = False) -> str | BinaryIO: + allow_unparseable_crs: bool = False, + drop_rotation: bool = False) -> str | BinaryIO: """Write data as a GeoTIFF or Cloud Optimized GeoTIFF. Tier: Stable for local-file output with ``compression`` in @@ -278,6 +280,20 @@ def to_geotiff(data: xr.DataArray | np.ndarray, protects against files where a typo'd PROJ string or an ``"EPSG:4326"`` token on a host without pyproj produces a citation that most readers cannot interpret. See issue #1929. + drop_rotation : bool, default False + Opt in to writing a DataArray that carries + ``attrs['rotated_affine']`` (issue #2216). The reader sets that + attr when called with ``allow_rotated=True`` on a file whose + ``ModelTransformationTag`` contains rotation, shear, or + z-coupling terms (issue #2129). The writer does not emit a + ``ModelTransformationTag`` (tracked in #2115), so passing such + a DataArray through ``to_geotiff`` produces an identity-affine + output and loses the rotated mapping; a subsequent + ``open_geotiff`` round-trip cannot recover it. Default + ``False`` refuses the write with ``ValueError`` so the loss is + impossible without an explicit signal. ``drop_rotation=True`` + accepts the loss and lets the write proceed; consumers reading + the output will see an axis-aligned, non-rotated TIFF. Returns ------- @@ -296,6 +312,13 @@ def to_geotiff(data: xr.DataArray | np.ndarray, (``b != 0`` or ``d != 0`` in rasterio ``Affine`` ordering). The on-disk GeoTIFF is axis-aligned; reproject onto an axis-aligned grid first. + ValueError + If ``data.attrs['rotated_affine']`` is set and ``drop_rotation`` + is False. The reader sets that attr on the ``allow_rotated=True`` + path; the writer cannot emit a ``ModelTransformationTag`` + (#2115) so writing would silently lose the rotation. Pass + ``drop_rotation=True`` to accept the loss explicitly + (issue #2216). ValueError If ``data`` is a 3D DataArray whose ``dims`` is not ``(band, y, x)`` or ``(y, x, band)`` (accepting the band-name @@ -330,6 +353,20 @@ def to_geotiff(data: xr.DataArray | np.ndarray, _validate_nodata_arg(nodata) + # Issue #2216: refuse to silently drop the rotated 6-tuple that the + # reader surfaces on ``attrs['rotated_affine']`` when called with + # ``allow_rotated=True``. The writer has no ``ModelTransformationTag`` + # emit path (#2115), so writing a rotated input produces an + # identity-affine file with no signal to the caller. Run the check + # at the entry point so the GPU dispatch path, the VRT path, and the + # eager path all hit the same gate. + _drop_rotation_attrs = getattr(data, 'attrs', None) or {} + _validate_no_rotated_affine( + _drop_rotation_attrs, + drop_rotation=drop_rotation, + entry_point="to_geotiff", + ) + # Issue #2075: reject zero-height / zero-width inputs before any # dispatch decision. Clip / window pipelines naturally produce empty # rasters and the writers used to accept them, produce a TIFF whose @@ -534,7 +571,8 @@ def to_geotiff(data: xr.DataArray | np.ndarray, max_z_error=max_z_error, photometric=photometric, allow_unparseable_crs=allow_unparseable_crs, - allow_internal_only_jpeg=allow_internal_only_jpeg) + allow_internal_only_jpeg=allow_internal_only_jpeg, + drop_rotation=drop_rotation) return path # Dispatch to write_geotiff_gpu when GPU was selected (explicit @@ -581,6 +619,7 @@ def to_geotiff(data: xr.DataArray | np.ndarray, allow_internal_only_jpeg=allow_internal_only_jpeg, allow_experimental_codecs=allow_experimental_codecs, allow_unparseable_crs=allow_unparseable_crs, + drop_rotation=drop_rotation, ) return path except ImportError as e: @@ -951,7 +990,8 @@ def _write_vrt_tiled(data, vrt_path, *, crs=None, nodata=None, bigtiff=None, max_z_error: float = 0.0, photometric: str | int = 'auto', allow_unparseable_crs: bool = False, - allow_internal_only_jpeg: bool = False): + allow_internal_only_jpeg: bool = False, + drop_rotation: bool = False): """Write a DataArray as a directory of tiled GeoTIFFs with a VRT index. This enables streaming dask arrays to disk without materializing the @@ -959,6 +999,18 @@ def _write_vrt_tiled(data, vrt_path, *, crs=None, nodata=None, """ _validate_nodata_arg(nodata) + # Mirror to_geotiff's rotated_affine gate (#2216) so the VRT path + # rejects rotated inputs at the same boundary. ``to_geotiff`` already + # ran the check upstream when the caller reached this branch through + # the dispatcher, but a direct call to ``_write_vrt_tiled`` would + # otherwise bypass the gate. + _drop_rotation_attrs = getattr(data, 'attrs', None) or {} + _validate_no_rotated_affine( + _drop_rotation_attrs, + drop_rotation=drop_rotation, + entry_point="to_geotiff", + ) + # Issue #1987 ambiguous-metadata checks; mirrors the call in # ``to_geotiff`` so the dask-VRT write path enforces the same # crs/crs_wkt / nodata / coord rules. diff --git a/xrspatial/geotiff/_writers/gpu.py b/xrspatial/geotiff/_writers/gpu.py index b4fdf1abb..efbdfdf33 100644 --- a/xrspatial/geotiff/_writers/gpu.py +++ b/xrspatial/geotiff/_writers/gpu.py @@ -34,6 +34,7 @@ from .._runtime import GeoTIFFFallbackWarning from .._validation import ( _validate_3d_writer_dims, + _validate_no_rotated_affine, _validate_nodata_arg, _validate_tile_size_arg, _validate_writer_spatial_shape, @@ -85,7 +86,8 @@ def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray | np.ndarray, photometric: str | int = 'auto', allow_internal_only_jpeg: bool = False, allow_experimental_codecs: bool = False, - allow_unparseable_crs: bool = False + allow_unparseable_crs: bool = False, + drop_rotation: bool = False, ) -> str | BinaryIO: """Write a CuPy-backed DataArray as a GeoTIFF with GPU compression. @@ -241,6 +243,14 @@ def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray | np.ndarray, ``GTCitationGeoKey`` (default ``False``). See :func:`to_geotiff` for the full description; the GPU writer applies the same fail-closed default. See issue #1929. + drop_rotation : bool, default False + Opt in to writing a DataArray that carries + ``attrs['rotated_affine']``. Mirrors the same kwarg on + ``to_geotiff`` so the two writers share one gate. Default + ``False`` refuses the write with ``ValueError``; the GPU + writer does not emit a ``ModelTransformationTag`` either + (tracked in #2115), so the silent-loss surface is identical + on both backends. See issue #2216. Returns ------- @@ -351,6 +361,16 @@ def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray | np.ndarray, _validate_tile_size_arg(tile_size) _validate_nodata_arg(nodata) + # Issue #2216: refuse to silently drop ``attrs['rotated_affine']``. + # Mirror the gate ``to_geotiff`` runs upstream so direct callers of + # ``write_geotiff_gpu`` get the same rejection. + _drop_rotation_attrs = getattr(data, 'attrs', None) or {} + _validate_no_rotated_affine( + _drop_rotation_attrs, + drop_rotation=drop_rotation, + entry_point="write_geotiff_gpu", + ) + # Issue #2075: reject empty spatial shapes. ``write_geotiff_gpu`` is # a public entry point and direct callers (with cupy.ndarray or raw # numpy) do not flow through ``to_geotiff``'s guard, so check here diff --git a/xrspatial/geotiff/tests/test_to_geotiff_drop_rotation_2216.py b/xrspatial/geotiff/tests/test_to_geotiff_drop_rotation_2216.py new file mode 100644 index 000000000..522a777b0 --- /dev/null +++ b/xrspatial/geotiff/tests/test_to_geotiff_drop_rotation_2216.py @@ -0,0 +1,270 @@ +"""``to_geotiff`` refuses to silently drop ``attrs['rotated_affine']``. + +Issue #2216. The reader exposes the rotated 6-tuple on +``attrs['rotated_affine']`` when called with ``allow_rotated=True`` +(issue #2129). The writer does not emit a ``ModelTransformationTag`` +(tracked in issue #2115), so the round-trip used to write an +identity-affine output without warning. This file pins the fail-closed +contract: + +* writing a rotated-affine raster without the opt-in raises + ``ValueError`` naming the attr; +* ``drop_rotation=True`` lets the write proceed and the round-trip + output carries no ``rotated_affine`` attr; +* writes of plain rasters with no rotated attr are unchanged; +* a read-then-write-then-read cycle on a rotated file requires the + opt-in to succeed. + +The tests target the eager ``to_geotiff`` entry point and reuse the +synthetic rotated-TIFF helper from ``test_rotated_affine_attr_2129.py`` +so the two suites stay in lockstep on what the reader produces. +""" +from __future__ import annotations + +import numpy as np +import pytest +import xarray as xr + +from xrspatial.geotiff import open_geotiff, to_geotiff +from xrspatial.geotiff._writers.eager import _write_vrt_tiled + + +_ROTATED_TUPLE = (8.66, -5.0, 100.0, 5.0, 8.66, 200.0) + + +def _write_rotated_tiff(path, arr, *, epsg=None): + """Write a synthetic rotated GeoTIFF (30-degree rotation).""" + tifffile = pytest.importorskip("tifffile") + 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, + ) + extratags = [(34264, 12, 16, m, False)] + if epsg is not None: + geo_key_directory = ( + 1, 1, 0, 1, + 2048, 0, 1, int(epsg), + ) + extratags.append((34735, 3, 8, geo_key_directory, False)) + tifffile.imwrite( + path, arr, photometric='minisblack', planarconfig='contig', + extratags=extratags, + ) + return m + + +def _rotated_dataarray(): + """Build a 2D DataArray that mimics ``open_geotiff(..., allow_rotated=True)``. + + Avoids depending on the synthetic-TIFF helper for the parsing-side + tests so the rejection logic is exercised on the boundary regardless + of whether the synthetic file produced the exact attrs the reader + would emit. + """ + arr = np.arange(20, dtype=np.float32).reshape(4, 5) + return xr.DataArray( + arr, + dims=('y', 'x'), + coords={'y': np.arange(4), 'x': np.arange(5)}, + attrs={'rotated_affine': _ROTATED_TUPLE}, + ) + + +# --------------------------------------------------------------------------- +# Default fail-closed gate. +# --------------------------------------------------------------------------- + + +def test_to_geotiff_rejects_rotated_affine_without_opt_in(tmp_path): + da = _rotated_dataarray() + out = tmp_path / "tmp_2216_reject.tif" + + with pytest.raises(ValueError, match="rotated_affine"): + to_geotiff(da, str(out)) + + # The error names the opt-in so the caller learns the flag. + with pytest.raises(ValueError, match="drop_rotation=True"): + to_geotiff(da, str(out)) + + # Nothing got written. + assert not out.exists() + + +def test_to_geotiff_error_message_points_at_issue(tmp_path): + """The rejection message references issue #2216 so a grep + ties back to this PR. The check is on the issue number, not on + surrounding phrasing, so the wording can evolve without breaking + the test.""" + da = _rotated_dataarray() + out = tmp_path / "tmp_2216_issue_ref.tif" + + with pytest.raises(ValueError, match="#2216"): + to_geotiff(da, str(out)) + + +# --------------------------------------------------------------------------- +# Opt-in path. +# --------------------------------------------------------------------------- + + +def test_to_geotiff_drop_rotation_writes_axis_aligned_file(tmp_path): + """``drop_rotation=True`` lets the write proceed and the output is a + plain axis-aligned (non-rotated) TIFF -- the round-trip reader sees + no ``rotated_affine`` attr.""" + da = _rotated_dataarray() + out = tmp_path / "tmp_2216_drop.tif" + + to_geotiff(da, str(out), drop_rotation=True) + + # Sanity: the file exists and re-opens. + assert out.exists() + da2 = open_geotiff(str(out)) + + # The rotated attr is gone on the round-trip (the writer has no + # ModelTransformationTag emit path, so the on-disk file is + # axis-aligned; the reader's normal path therefore sees no + # rotated-tag and emits no rotated_affine attr). + assert 'rotated_affine' not in da2.attrs + + +def test_to_geotiff_drop_rotation_preserves_pixel_values(tmp_path): + """The opt-in only drops the rotated *georeferencing*; the pixel + grid itself round-trips unchanged.""" + da = _rotated_dataarray() + out = tmp_path / "tmp_2216_pixels.tif" + + to_geotiff(da, str(out), drop_rotation=True) + da2 = open_geotiff(str(out)) + + np.testing.assert_array_equal( + np.asarray(da2.data, dtype=np.float32), + np.asarray(da.data, dtype=np.float32), + ) + + +# --------------------------------------------------------------------------- +# Non-rotated raster baseline. +# --------------------------------------------------------------------------- + + +def test_to_geotiff_normal_raster_unchanged(tmp_path): + """A DataArray with no ``rotated_affine`` attr writes the same way + it always did -- the new gate must not change behaviour for the + common path.""" + arr = np.arange(12, dtype=np.float32).reshape(3, 4) + da = xr.DataArray( + arr, + dims=('y', 'x'), + coords={'y': np.arange(3), 'x': np.arange(4)}, + attrs={}, + ) + out = tmp_path / "tmp_2216_normal.tif" + + to_geotiff(da, str(out)) + assert out.exists() + + # And explicitly setting ``drop_rotation=True`` on a non-rotated + # input is a no-op; the kwarg only affects the rotated path. + out2 = tmp_path / "tmp_2216_normal_optin.tif" + to_geotiff(da, str(out2), drop_rotation=True) + assert out2.exists() + + +def test_to_geotiff_rotated_affine_none_does_not_trigger(tmp_path): + """A literal ``attrs['rotated_affine'] = None`` does not trigger + the gate. The check is on the attr's truthiness so a future read + path that pre-allocates the key with ``None`` does not break the + common write path.""" + arr = np.arange(6, dtype=np.float32).reshape(2, 3) + da = xr.DataArray( + arr, dims=('y', 'x'), + coords={'y': np.arange(2), 'x': np.arange(3)}, + attrs={'rotated_affine': None}, + ) + out = tmp_path / "tmp_2216_none.tif" + + to_geotiff(da, str(out)) + assert out.exists() + + +# --------------------------------------------------------------------------- +# End-to-end round-trip: opening a real rotated TIFF and writing back. +# --------------------------------------------------------------------------- + + +def test_round_trip_rotated_tiff_requires_opt_in(tmp_path): + """Read a rotated TIFF with ``allow_rotated=True``, then attempt to + write it back. Without the opt-in the write raises; with the opt-in + it succeeds and the round-trip output is axis-aligned.""" + arr = np.arange(20, dtype=' Date: Wed, 20 May 2026 19:51:54 -0700 Subject: [PATCH 2/2] Address review nits: helper name + empty-tuple comment (#2216) - ``_write_vrt_tiled`` now passes ``entry_point="_write_vrt_tiled"`` to the rotated-affine validator so a direct caller of the private helper sees the actual entry point in the error message. The public ``to_geotiff`` dispatch already runs its own gate upstream, so the helper-name surface is only visible to direct callers. - Add a one-line comment in ``_validate_no_rotated_affine`` explaining the ``is None`` semantics: missing key and explicit None both skip, and any other value (including an empty tuple) falls through to the rejection branch so a malformed marker fails closed. Test updated to assert the helper name surfaces in the direct-call rejection message. --- xrspatial/geotiff/_validation.py | 6 ++++++ xrspatial/geotiff/_writers/eager.py | 8 ++++++-- .../geotiff/tests/test_to_geotiff_drop_rotation_2216.py | 6 ++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/xrspatial/geotiff/_validation.py b/xrspatial/geotiff/_validation.py index 0bcb35c0d..d8aa98278 100644 --- a/xrspatial/geotiff/_validation.py +++ b/xrspatial/geotiff/_validation.py @@ -610,6 +610,12 @@ def _validate_no_rotated_affine(attrs, *, drop_rotation: bool, """ if not attrs: return + # ``is None`` skips both the missing-key case and the explicit-None + # case (the reader's ``_populate_attrs_from_geo_info`` only sets the + # attr to a tuple OR omits it; an explicit ``None`` arises from + # caller code that pre-allocates the key). Any other value -- tuple, + # empty tuple, list, ndarray -- falls through to the rejection + # branch so a malformed marker still fails closed. if attrs.get('rotated_affine') is None: return if drop_rotation: diff --git a/xrspatial/geotiff/_writers/eager.py b/xrspatial/geotiff/_writers/eager.py index 235b99a4f..0f1a2b392 100644 --- a/xrspatial/geotiff/_writers/eager.py +++ b/xrspatial/geotiff/_writers/eager.py @@ -1003,12 +1003,16 @@ def _write_vrt_tiled(data, vrt_path, *, crs=None, nodata=None, # rejects rotated inputs at the same boundary. ``to_geotiff`` already # ran the check upstream when the caller reached this branch through # the dispatcher, but a direct call to ``_write_vrt_tiled`` would - # otherwise bypass the gate. + # otherwise bypass the gate. ``entry_point`` names the private helper + # so a direct caller sees the function actually running the check; + # the public ``to_geotiff`` dispatch path raised at its own gate + # before reaching this point so the helper-name surface is only + # visible to direct callers (review nit on #2216). _drop_rotation_attrs = getattr(data, 'attrs', None) or {} _validate_no_rotated_affine( _drop_rotation_attrs, drop_rotation=drop_rotation, - entry_point="to_geotiff", + entry_point="_write_vrt_tiled", ) # Issue #1987 ambiguous-metadata checks; mirrors the call in diff --git a/xrspatial/geotiff/tests/test_to_geotiff_drop_rotation_2216.py b/xrspatial/geotiff/tests/test_to_geotiff_drop_rotation_2216.py index 522a777b0..2009c38f3 100644 --- a/xrspatial/geotiff/tests/test_to_geotiff_drop_rotation_2216.py +++ b/xrspatial/geotiff/tests/test_to_geotiff_drop_rotation_2216.py @@ -268,3 +268,9 @@ def test_write_vrt_tiled_direct_call_rejects_rotated_affine(tmp_path): with pytest.raises(ValueError, match="rotated_affine"): _write_vrt_tiled(da, str(vrt_out)) + + # The error names the function actually running the check, not the + # public wrapper, so a direct caller of the private helper learns + # which entry point fired (review nit on #2216). + with pytest.raises(ValueError, match="_write_vrt_tiled"): + _write_vrt_tiled(da, str(vrt_out))