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
27 changes: 18 additions & 9 deletions xrspatial/geotiff/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2480,21 +2480,30 @@ def _apply_orientation_geo_info(geo_info, orientation: int,
pixel_height=new_px_h,
)
elif orientation in (5, 6, 7, 8):
# Match the CPU reader's #1765 refusal: a pixel-size swap alone
# cannot express the per-orientation origin shift plus rotation
# these orientations require, so the x/y coords would be wrong.
# ``has_georef`` is True for any file carrying ModelTransformation,
# ModelPixelScale, or ModelTiepoint, with or without a CRS tag, so
# gate on that flag rather than CRS presence.
if getattr(geo_info, 'has_georef', False):
raise NotImplementedError(
f"TIFF Orientation {orientation} on a georeferenced file "
f"requires a per-orientation origin shift plus a rotation "
f"that the axis-aligned GeoTransform used here cannot "
f"represent, so the returned x/y coords would be wrong. "
f"Reproject the file with another tool (e.g. GDAL) or "
f"strip the Orientation tag before reading. See issue "
f"#1765."
)
# Non-georeferenced file: swap pixel sizes to match the
# transposed array shape. No geographic claim to violate.
geo_info.transform = GeoTransform(
origin_x=t.origin_x,
origin_y=t.origin_y,
pixel_width=t.pixel_height,
pixel_height=t.pixel_width,
)
if (geo_info.crs_epsg is not None
or geo_info.crs_wkt is not None):
warnings.warn(
f"Orientation {orientation} swaps spatial axes on "
f"a georeferenced file; the returned coords are "
f"shape-correct but the geographic transform may "
f"need manual adjustment.",
stacklevel=3,
)
return geo_info


Expand Down
30 changes: 15 additions & 15 deletions xrspatial/geotiff/_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -2258,22 +2258,22 @@ def _apply_orientation_with_geo(
pixel_height=new_px_h,
)
elif orientation in (5, 6, 7, 8):
geo_info.transform = GeoTransform(
origin_x=t.origin_x,
origin_y=t.origin_y,
pixel_width=t.pixel_height,
pixel_height=t.pixel_width,
# ``has_georef`` is True whenever ModelTransformation,
# ModelPixelScale, or ModelTiepoint is present, even without a
# CRS. The pixel-size swap below cannot express the
# per-orientation origin shift plus rotation these orientations
# require, so the x/y coords would be wrong whether or not a
# CRS tag accompanies the transform. Refuse the file in that
# case rather than warn and return silently wrong coords.
raise NotImplementedError(
f"TIFF Orientation {orientation} on a georeferenced file "
f"requires a per-orientation origin shift plus a rotation "
f"that the axis-aligned GeoTransform used here cannot "
f"represent, so the returned x/y coords would be wrong. "
f"Reproject the file with another tool (e.g. GDAL) or "
f"strip the Orientation tag before reading. See issue "
f"#1765."
)
if (geo_info.crs_epsg is not None
or geo_info.crs_wkt is not None):
import warnings
warnings.warn(
f"Orientation {orientation} swaps spatial axes on "
f"a georeferenced file; the returned coords are "
f"shape-correct but the geographic transform may "
f"need manual adjustment.",
stacklevel=2,
)
return arr, geo_info


Expand Down
107 changes: 83 additions & 24 deletions xrspatial/geotiff/tests/test_orientation.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,47 +193,106 @@ def test_orientation_round_trip_does_not_double_apply(tmp_path):


@pytest.mark.parametrize("orientation", [5, 6, 7, 8])
def test_orientation_5_to_8_warn_on_georef(tmp_path, orientation):
"""Axis-swap orientations on georef'd files emit a warning.

The transform swap is shape-correct but geometrically approximate
for orientations 6/7/8; the user should be told.
def test_orientation_5_to_8_raise_on_georef(tmp_path, orientation):
"""Axis-swap orientations on georef'd files raise NotImplementedError.

Orientations 5-8 require a per-orientation origin shift plus a
rotation that the axis-aligned GeoTransform cannot represent.
The reader used to swap pixel_width/pixel_height and warn; that
produced silently wrong coords on georef'd files (issue #1765).
The reader now refuses the file instead.
"""
import warnings as _warnings

arr = np.arange(24, dtype=np.uint8).reshape(4, 6)
path = tmp_path / f"orient_georef_{orientation}.tif"
# tifffile lets us tag a CRS via the resolution + metadata; the
# simplest path that triggers our georef branch is to embed a
# ModelPixelScale + GeoKeyDirectory pair pointing at EPSG:4326.
path = tmp_path / f"orient_georef_raise_1765_{orientation}.tif"
# ModelPixelScale + GeoKeyDirectory pair pointing at EPSG:4326
# makes the reader treat this as a georeferenced file.
tifffile.imwrite(
str(path), arr,
extratags=[
(274, 'H', 1, orientation, True),
# ModelPixelScaleTag (33550): scale_x, scale_y, scale_z
(33550, 'd', 3, (1.0, 1.0, 0.0), True),
# ModelTiepointTag (33922): I, J, K, X, Y, Z
(33922, 'd', 6, (0.0, 0.0, 0.0, 0.0, 0.0, 0.0), True),
# GeoKeyDirectoryTag (34735): version 1.1.0, 1 key,
# GTModelType=2 (Geographic), GeographicType=4326
(34735, 'H', 12, (
1, 1, 0, 2,
1024, 0, 1, 2, # GTModelTypeGeoKey = 2
2048, 0, 1, 4326, # GeographicTypeGeoKey = 4326
1024, 0, 1, 2,
2048, 0, 1, 4326,
), True),
],
)

with _warnings.catch_warnings(record=True) as caught:
_warnings.simplefilter('always')
da = open_geotiff(str(path))
with pytest.raises(NotImplementedError, match=str(orientation)):
open_geotiff(str(path))
Comment on lines 195 to +224
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, GPU was still going through _apply_orientation_geo_info, which warned and returned the same wrong coords #1765 was meant to close. Updated the GPU helper to raise NotImplementedError for has_georef=True orientation 5-8 with the same message as the CPU path. Added three new GPU tests: a raise-on-georef parametrize, a raise-on-transform-only parametrize, and a regression guard that no-georef files still swap axes. All 40 tests in test_orientation_gpu.py pass.



@pytest.mark.parametrize("orientation", [5, 6, 7, 8])
def test_orientation_5_to_8_transform_only_raises(tmp_path, orientation):
"""``has_georef`` without CRS still triggers the raise.

A TIFF carrying ModelPixelScale + ModelTiepoint but no
GeoKeyDirectory has ``has_georef=True`` and ``crs_epsg=None``. The
pixel-size swap alone misses the per-orientation origin shift, so
refusing is the honest contract regardless of CRS tagging.
"""
arr = np.arange(24, dtype=np.uint8).reshape(4, 6)
path = tmp_path / f"orient_transform_only_1765_{orientation}.tif"
tifffile.imwrite(
str(path), arr,
extratags=[
(274, 'H', 1, orientation, True),
(33550, 'd', 3, (1.0, 1.0, 0.0), True),
(33922, 'd', 6, (0.0, 0.0, 0.0, 0.0, 0.0, 0.0), True),
],
)

with pytest.raises(NotImplementedError, match=str(orientation)):
open_geotiff(str(path))


@pytest.mark.parametrize("orientation", [5, 6, 7, 8])
def test_orientation_5_to_8_no_geo_still_swaps(tmp_path, orientation):
"""Without georef, orientations 5-8 still do the axis swap.

No geographic claim to violate, so the existing transpose path is
preserved (regression guard for the #1765 fix not over-reaching).
"""
arr = np.arange(24, dtype=np.uint8).reshape(4, 6)
path = tmp_path / f"orient_no_geo_1765_{orientation}.tif"
tifffile.imwrite(
str(path), arr,
extratags=[(274, 'H', 1, orientation, True)],
)

da = open_geotiff(str(path))
expected = _expected_for_orientation(arr, orientation)
assert da.values.shape == expected.shape
np.testing.assert_array_equal(da.values, expected)


assert da is not None
msgs = [str(w.message) for w in caught if 'Orientation' in str(w.message)]
assert msgs, (
f"orientation={orientation}: no warning emitted on georef file"
def test_orientation_1_georef_unchanged_1765(tmp_path):
"""orientation=1 on a georef'd file still reads normally.

Regression guard: the #1765 raise must be scoped to 5-8, not fire
on any georeferenced file.
"""
arr = np.arange(24, dtype=np.uint8).reshape(4, 6)
path = tmp_path / "orient_georef_1_1765.tif"
tifffile.imwrite(
str(path), arr,
extratags=[
(274, 'H', 1, 1, True),
(33550, 'd', 3, (1.0, 1.0, 0.0), True),
(33922, 'd', 6, (0.0, 0.0, 0.0, 0.0, 0.0, 0.0), True),
(34735, 'H', 12, (
1, 1, 0, 2,
1024, 0, 1, 2,
2048, 0, 1, 4326,
), True),
],
)

da = open_geotiff(str(path))
np.testing.assert_array_equal(da.values, arr)


# ---------------------------------------------------------------------------
# Geographic coordinate updates for mirror-flip orientations (issue #1537)
Expand Down
75 changes: 75 additions & 0 deletions xrspatial/geotiff/tests/test_orientation_gpu.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,78 @@ def test_gpu_default_orientation_unchanged(tmp_path):

da = read_geotiff_gpu(str(path))
np.testing.assert_array_equal(da.data.get(), arr)


@_gpu_only
@pytest.mark.parametrize("orientation", [5, 6, 7, 8])
def test_gpu_orientation_5_to_8_raise_on_georef(tmp_path, orientation):
"""GPU reader refuses georef'd files with axis-swap orientations.

Mirrors the CPU behaviour added for issue #1765. ``read_geotiff_gpu``
used to warn and return silently wrong x/y coords for these cases.
"""
from xrspatial.geotiff import read_geotiff_gpu

arr = np.arange(256, dtype=np.uint8).reshape(16, 16)
path = tmp_path / f"gpu_orient_georef_raise_1765_{orientation}.tif"
_write_tiled(
path, arr, orientation,
extra=[
(33550, 'd', 3, (1.0, 1.0, 0.0), True),
(33922, 'd', 6, (0.0, 0.0, 0.0, 100.0, 50.0, 0.0), True),
(34735, 'H', 12, (
1, 1, 0, 2,
1024, 0, 1, 2,
2048, 0, 1, 4326,
), True),
],
)

with pytest.raises(NotImplementedError, match=str(orientation)):
read_geotiff_gpu(str(path))


@_gpu_only
@pytest.mark.parametrize("orientation", [5, 6, 7, 8])
def test_gpu_orientation_5_to_8_transform_only_raises(tmp_path, orientation):
"""``has_georef`` without CRS still triggers the raise on GPU.

Files carrying ModelPixelScale + ModelTiepoint but no
GeoKeyDirectory have ``has_georef=True``/``crs_epsg=None``. The
pixel-size swap alone misses the per-orientation origin shift, so
refusing is the honest contract regardless of CRS tagging.
"""
from xrspatial.geotiff import read_geotiff_gpu

arr = np.arange(256, dtype=np.uint8).reshape(16, 16)
path = tmp_path / f"gpu_orient_transform_only_1765_{orientation}.tif"
_write_tiled(
path, arr, orientation,
extra=[
(33550, 'd', 3, (1.0, 1.0, 0.0), True),
(33922, 'd', 6, (0.0, 0.0, 0.0, 100.0, 50.0, 0.0), True),
],
)

with pytest.raises(NotImplementedError, match=str(orientation)):
read_geotiff_gpu(str(path))


@_gpu_only
@pytest.mark.parametrize("orientation", [5, 6, 7, 8])
def test_gpu_orientation_5_to_8_no_georef_still_swaps(tmp_path, orientation):
"""Without any geo tags, GPU orientation 5-8 still swaps axes.

Regression guard for the #1765 GPU fix: refusing must be scoped to
georeferenced files, not every Orientation 5-8 read.
"""
from xrspatial.geotiff import read_geotiff_gpu

arr = np.arange(256, dtype=np.uint8).reshape(16, 16)
path = tmp_path / f"gpu_orient_no_geo_1765_{orientation}.tif"
_write_tiled(path, arr, orientation)

da = read_geotiff_gpu(str(path))
expected = _expected_for_orientation(arr, orientation)
assert da.data.shape == expected.shape
np.testing.assert_array_equal(da.data.get(), expected)
Loading