Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions xrspatial/geotiff/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,9 +446,10 @@ def open_geotiff(source: str | BinaryIO, *,
pixel, and ``dtype=<integer>`` 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.
Expand All @@ -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
-------
Expand Down
10 changes: 6 additions & 4 deletions xrspatial/geotiff/_geotags.py
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
48 changes: 48 additions & 0 deletions xrspatial/geotiff/_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,54 @@ 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
# ``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:
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)
#
Expand Down
62 changes: 59 additions & 3 deletions xrspatial/geotiff/_writers/eager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
-------
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -951,14 +990,31 @@ 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
full array in RAM.
"""
_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. ``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="_write_vrt_tiled",
)

# 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.
Expand Down
22 changes: 21 additions & 1 deletion xrspatial/geotiff/_writers/gpu.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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
-------
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading