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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
- Reject writes whose `attrs['crs']` and `attrs['crs_wkt']` resolve to different CRSes (after pyproj canonicalisation) instead of silently emitting the EPSG and dropping the WKT. The new `ConflictingCRSError` (subclass of `ValueError` and `GeoTIFFAmbiguousMetadataError`) names the offending attrs; pass `crs=` explicitly to override both attrs and bypass the check. Read-back DataArrays carrying both attrs continue to round-trip because the reader's two attrs derive from the same on-disk CRS. (#1987)
- Reject reads whose CRS string cannot be parsed by pyproj instead of emitting it verbatim in `attrs['crs_wkt']` and letting downstream code crash on first use. Raises `UnparseableCRSError` (subclass of `ValueError`); pass `allow_unparseable_crs=True` to `open_geotiff` / `read_geotiff_dask` / `read_geotiff_gpu` / `read_vrt` to keep the legacy behaviour. The existing write-side raise from `_validate_crs_fallback` was retyped from plain `ValueError` to the new `UnparseableCRSError` subclass (no behaviour change). (#1987)
- Reject reads whose affine transform has non-zero rotation/shear terms instead of returning an axis-misaligned grid that downstream xrspatial ops (slope, aspect, hillshade, proximity, zonal) silently compute wrong results on. Raises `RotatedTransformError`; pass `allow_rotated=True` to `open_geotiff` / `read_geotiff_dask` / `read_geotiff_gpu` / `read_vrt` to read the pixel grid without the axis-aligned-grid assumption. (#1987)
- Extend `RotatedTransformError` to the plain GeoTIFF read path. Previously the VRT path raised the typed error via `_check_read_rotated_transform`, but a rotated `ModelTransformationTag` on a plain GeoTIFF raised a bare `NotImplementedError` from the parser, so `except RotatedTransformError` (or `except GeoTIFFAmbiguousMetadataError`) only caught the VRT case. Both entry points now raise the same typed error. `RotatedTransformError` subclasses `ValueError`, so callers catching `ValueError` are unaffected; callers that caught `NotImplementedError` specifically for the rotated branch should switch to `RotatedTransformError` (or `ValueError`). (#2267)
- Reject writes whose `coords['y']` or `coords['x']` are not uniformly spaced instead of silently using the first two values as the pixel size and misrepresenting the rest of the axis. Raises `NonUniformCoordsError`; the existing int-dtype sentinel convention from #1969 (used by the no-georef coord fallback) is exempted. (#1987)
- Reject writes whose `attrs['nodata']` disagrees with every concrete entry in `attrs['nodatavals']` instead of silently picking the canonical scalar and dropping the rioxarray tuple. Raises `ConflictingNodataError`; pass `nodata=` explicitly to the writer to override both attrs and bypass the check. `_FillValue` continues to be deprioritised per the existing resolver convention. (#1987)

Expand Down
9 changes: 6 additions & 3 deletions xrspatial/geotiff/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ def _read_geo_info(source, *, overview_level: int | None = None,
allow_rotated : bool, optional
Forwarded to the geotag parser. When True, a rotated
``ModelTransformationTag`` reads as an ungeoreferenced pixel
grid instead of raising ``NotImplementedError`` (issue #2115).
grid instead of raising ``RotatedTransformError`` (issues #2115,
#2267).
"""
from ._dtypes import resolve_bits_per_sample, tiff_dtype_to_numpy
from ._geotags import extract_geo_info_with_overview_inheritance
Expand Down Expand Up @@ -471,8 +472,10 @@ def open_geotiff(source: str | BinaryIO, *,
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.
files. By default the reader raises ``RotatedTransformError``
(a ``GeoTIFFAmbiguousMetadataError`` / ``ValueError`` subclass;
previously a bare ``NotImplementedError`` -- see #2267) because
the rest of xrspatial assumes an axis-aligned grid.
``allow_rotated=True`` reads the pixel grid without the
geospatial assumption: the result has integer pixel coords on
``x`` / ``y`` and both ``attrs['crs']`` and ``attrs['crs_wkt']``
Expand Down
12 changes: 8 additions & 4 deletions xrspatial/geotiff/_attrs.py
Original file line number Diff line number Diff line change
Expand Up @@ -814,10 +814,14 @@ def _validate_read_geo_info(
(``b == 0`` / ``d == 0``) because ``_transform_tuple_from_pixel_geometry``
only carries origin + pixel size, and the upstream TIFF reader
rejects rotated ``ModelTransformationTag`` entries with
``NotImplementedError`` in ``_geotags._extract_transform_and_georef``
before we reach this helper. The rotated-transform check therefore
fires only on the VRT path, which builds its context from the GDAL
``geo_transform`` via ``_gdal_geotransform_to_affine_tuple``.
``RotatedTransformError`` in ``_geotags._extract_transform_and_georef``
before we reach this helper (issue #2267; previously
``NotImplementedError``). Both the GeoTIFF and VRT entry points
therefore raise the same typed error -- the rotated-transform check
in this helper still fires only on the VRT path (which builds its
context from the GDAL ``geo_transform`` via
``_gdal_geotransform_to_affine_tuple``), but the contract a caller
sees is identical across both.
"""
from ._validation import validate_read_metadata
transform_for_check = (
Expand Down
3 changes: 2 additions & 1 deletion xrspatial/geotiff/_backends/dask.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ def read_geotiff_dask(source: str, *,
# rotated opt-in the same way as the eager HTTP path and the
# local chunked path (#2130). Without this, rotated remote
# GeoTIFFs raised ``NotImplementedError`` from
# ``_parse_cog_http_meta`` even when the caller had passed
# ``_parse_cog_http_meta`` (now ``RotatedTransformError``
# after #2267) even when the caller had passed
# ``allow_rotated=True``.
#
# ``source_path=source`` and ``return_sidecar=True`` opt the
Expand Down
20 changes: 13 additions & 7 deletions xrspatial/geotiff/_geotags.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
TAG_GEO_KEY_DIRECTORY, TAG_GEO_DOUBLE_PARAMS, TAG_GEO_ASCII_PARAMS,
)
from ._dtypes import resolve_bits_per_sample
from ._errors import RotatedTransformError

# ImageDescription tag (270). Captured for round-trip but not managed
# by the writer -- it flows through extra_tags pass-through.
Expand Down Expand Up @@ -630,7 +631,8 @@ def _extract_transform(ifd: IFD,
``Affine`` ordering: ``(pixel_width, b, origin_x, d,
pixel_height, origin_y)``); read it directly when the rotated
mapping is needed. Default ``False`` -- existing behaviour,
raise ``NotImplementedError``.
raise ``RotatedTransformError`` (issue #2267; previously
``NotImplementedError``).

This contract is read-only. ``rotated_affine`` is not currently
emitted by the writer. As of issue #2216 the writer refuses
Expand Down Expand Up @@ -660,10 +662,13 @@ def _extract_transform(ifd: IFD,
# y = M[4]*col + M[5]*row + M[6]*z + M[7]
#
# GeoTransform only carries the axis-aligned case. For rotated, sheared,
# or z-coupled transforms we raise NotImplementedError unless the caller
# opts out via ``allow_rotated`` (issue #2115). The opt-out drops the
# georef so downstream coord generation uses pixel indices and any
# spatial op that runs on the array sees no geo assumption to violate.
# or z-coupled transforms we raise ``RotatedTransformError`` unless the
# caller opts out via ``allow_rotated`` (issues #2115, #2267). The opt-out
# drops the georef so downstream coord generation uses pixel indices and
# any spatial op that runs on the array sees no geo assumption to violate.
# ``RotatedTransformError`` is the same typed error the VRT path raises
# via ``_check_read_rotated_transform`` in ``_validation.py``, so both
# entry points share one ``except`` contract.
transform_tag = ifd.get_value(TAG_MODEL_TRANSFORMATION)
if transform_tag is not None:
if isinstance(transform_tag, tuple) and len(transform_tag) >= 12:
Expand All @@ -676,7 +681,7 @@ def _extract_transform(ifd: IFD,
z_terms = (m[2], m[6]) if len(m) >= 8 else (0.0, 0.0)
if any(abs(t) > tol for t in rotation_terms + z_terms):
if not allow_rotated:
raise NotImplementedError(
raise RotatedTransformError(
"ModelTransformationTag (34264) contains rotation, "
"skew, or z-coupling terms "
f"(M[1]={m[1]!r}, M[4]={m[4]!r}, "
Expand Down Expand Up @@ -837,7 +842,8 @@ def extract_geo_info(ifd: IFD, data: bytes | memoryview,
allow_rotated : bool, optional
Forwarded to :func:`_extract_transform`. When True, a rotated
``ModelTransformationTag`` is read as an ungeoreferenced pixel
grid instead of raising ``NotImplementedError`` (issue #2115).
grid instead of raising ``RotatedTransformError`` (issue #2115,
#2267).

Returns
-------
Expand Down
17 changes: 9 additions & 8 deletions xrspatial/geotiff/tests/test_allow_rotated_geotiff_2115.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
opt-out could be consulted. The VRT path honoured the kwarg; the
GeoTIFF path did not.

These tests pin the new behaviour:
These tests pin the current behaviour:

* Default (``allow_rotated=False``) still raises ``NotImplementedError``
so an existing caller that does not know about the opt-out keeps the
safety net.
* Default (``allow_rotated=False``) raises ``RotatedTransformError``
(issue #2267; previously ``NotImplementedError``) so an existing
caller that does not know about the opt-out keeps the safety net.
* ``allow_rotated=True`` drops the georef and reads the pixel grid
without raising. The pixel values match what the file actually holds.
* The rotated 6-tuple is preserved on ``geo_info.transform.rotated_affine``
Expand All @@ -25,6 +25,7 @@
import pytest

from xrspatial.geotiff import open_geotiff
from xrspatial.geotiff._errors import RotatedTransformError
from xrspatial.geotiff._geotags import (
GeoTransform,
TAG_MODEL_TRANSFORMATION,
Expand Down Expand Up @@ -66,7 +67,7 @@ def _make_ifd_with_rotated_transform(matrix_16: tuple) -> IFD:

def test_extract_transform_rejects_rotated_by_default():
ifd = _make_ifd_with_rotated_transform(_ROTATED_M)
with pytest.raises(NotImplementedError, match="rotation"):
with pytest.raises(RotatedTransformError, match="rotation"):
_extract_transform(ifd)


Expand Down Expand Up @@ -159,7 +160,7 @@ def test_open_geotiff_rotated_default_raises(tmp_path):
src = tmp_path / "rotated_2115_default.tif"
arr = np.arange(20, dtype='<u2').reshape(4, 5)
_write_rotated_tiff(str(src), arr)
with pytest.raises(NotImplementedError, match="rotation"):
with pytest.raises(RotatedTransformError, match="rotation"):
open_geotiff(str(src))


Expand All @@ -185,7 +186,7 @@ def test_open_geotiff_rotated_default_raises_with_dask(tmp_path):
src = tmp_path / "rotated_2115_dask_default.tif"
arr = np.arange(20, dtype='<u2').reshape(4, 5)
_write_rotated_tiff(str(src), arr)
with pytest.raises(NotImplementedError, match="rotation"):
with pytest.raises(RotatedTransformError, match="rotation"):
open_geotiff(str(src), chunks=2)


Expand Down Expand Up @@ -227,7 +228,7 @@ def test_open_geotiff_http_rotated_default_raises(tmp_path, monkeypatch):
httpd = _start_http_server(tmp_path)
try:
url = f"http://127.0.0.1:{httpd.server_address[1]}/{src.name}"
with pytest.raises(NotImplementedError, match="rotation"):
with pytest.raises(RotatedTransformError, match="rotation"):
open_geotiff(url)
finally:
httpd.shutdown()
Expand Down
3 changes: 2 additions & 1 deletion xrspatial/geotiff/tests/test_backend_parity_matrix.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import xarray as xr

from xrspatial.geotiff import open_geotiff, to_geotiff, write_vrt
from xrspatial.geotiff._errors import RotatedTransformError


# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -976,7 +977,7 @@ def _build_rotated_no_optin(dir_path: Path, target: Path) -> Path:
_ERROR_FIXTURES: list[_ErrorFixtureSpec] = [
_ErrorFixtureSpec(
fix_id="rotated-no-allow_rotated",
exc=NotImplementedError,
exc=RotatedTransformError,
match="rotation",
source_type=_SRC_LOCAL_TIFF,
builder=_build_rotated_no_optin,
Expand Down
3 changes: 2 additions & 1 deletion xrspatial/geotiff/tests/test_georef_status_2136.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import xarray as xr

from xrspatial.geotiff import open_geotiff, read_vrt, to_geotiff
from xrspatial.geotiff._errors import RotatedTransformError
from xrspatial.geotiff._attrs import (
GEOREF_STATUS_CRS_ONLY,
GEOREF_STATUS_FULL,
Expand Down Expand Up @@ -287,7 +288,7 @@ def test_rotated_default_still_raises(tmp_path):
path = str(tmp_path / "georef_status_2136_rotated_default.tif")
arr = np.arange(20, dtype='<u2').reshape(4, 5)
_write_rotated_tiff(path, arr)
with pytest.raises(NotImplementedError, match="rotation"):
with pytest.raises(RotatedTransformError, match="rotation"):
open_geotiff(path)


Expand Down
5 changes: 3 additions & 2 deletions xrspatial/geotiff/tests/test_geotags.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
TAG_MODEL_PIXEL_SCALE,
TAG_MODEL_TIEPOINT,
)
from xrspatial.geotiff._errors import RotatedTransformError
from xrspatial.geotiff._header import parse_all_ifds, parse_header
from .conftest import make_minimal_tiff

Expand Down Expand Up @@ -233,7 +234,7 @@ def test_rotation_raises(self, tmp_path):
path.write_bytes(data)

from xrspatial.geotiff._reader import read_to_array
with pytest.raises(NotImplementedError) as exc:
with pytest.raises(RotatedTransformError) as exc:
read_to_array(str(path))
assert 'ModelTransformationTag' in str(exc.value)
assert 'rotation' in str(exc.value).lower() or 'skew' in str(exc.value).lower()
Expand All @@ -251,7 +252,7 @@ def test_z_coupling_raises(self, tmp_path):
path.write_bytes(data)

from xrspatial.geotiff._reader import read_to_array
with pytest.raises(NotImplementedError):
with pytest.raises(RotatedTransformError):
read_to_array(str(path))


Expand Down
3 changes: 2 additions & 1 deletion xrspatial/geotiff/tests/test_http_dask_allow_rotated_2130.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import pytest

from xrspatial.geotiff import open_geotiff
from xrspatial.geotiff._errors import RotatedTransformError

tifffile = pytest.importorskip("tifffile")

Expand Down Expand Up @@ -101,7 +102,7 @@ def test_http_dask_rotated_default_raises(tmp_path, monkeypatch):
httpd, port = _serve(payload)
try:
url = f'http://127.0.0.1:{port}/{src.name}'
with pytest.raises(NotImplementedError, match="rotation"):
with pytest.raises(RotatedTransformError, match="rotation"):
open_geotiff(url, chunks=4)
finally:
httpd.shutdown()
Expand Down
Loading
Loading