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
7 changes: 6 additions & 1 deletion docs/source/user_guide/attrs_contract.rst
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,12 @@ write.
fake unit transform. Absence of the marker means the array
has spatial coords the writer can interpret as georef. A
caller can opt into no-georef writes on a hand-built array
by setting this attr explicitly. See issue #2120.
by setting this attr explicitly. The writer uses an identity
check (``attrs[_xrspatial_no_georef] is True``), so only the
exact boolean ``True`` flips the no-georef path; truthy
strings like ``'yes'`` or ``1`` are ignored and the writer
proceeds with normal transform synthesis. See issues #2120
and #2133.


Compatibility aliases
Expand Down
45 changes: 12 additions & 33 deletions xrspatial/geotiff/_coords.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
"""
from __future__ import annotations

from collections.abc import Mapping
from typing import Any

import numpy as np
import xarray as xr

Expand All @@ -36,7 +39,7 @@
_BAND_DIM_NAMES = ('band', 'bands', 'channel')


def _has_no_georef_marker(da: xr.DataArray) -> bool:
def _has_no_georef_marker(da: Any) -> bool:
"""True iff ``da`` was stamped by the reader as carrying no georef.

The reader sets ``attrs[_NO_GEOREF_KEY] = True`` whenever it emits
Expand All @@ -50,40 +53,16 @@ def _has_no_georef_marker(da: xr.DataArray) -> bool:
boolean ``True`` flips the writer into no-georef mode. A stray
third-party stamp like ``attrs['_xrspatial_no_georef'] = 'yes'``
should not be treated as truthy and silently drop a transform.

Inputs that are not xarray DataArrays (e.g. a raw ``numpy.ndarray``
or ``cupy.ndarray`` passed directly to ``write_geotiff_gpu``) carry
no attrs and therefore no marker; return ``False`` rather than
raise so callers can use this as a plain predicate.
"""
return da.attrs.get(_NO_GEOREF_KEY) is True


# Kept for diagnostic / test pinning only. The writer no longer
# calls this helper -- it checks ``attrs[_NO_GEOREF_KEY]`` instead
# (see :func:`_has_no_georef_marker` / issue #2120). Only the tests
# in ``test_int_coord_sentinel_2087.py`` reference it now.
def _is_no_georef_sentinel(coord: np.ndarray) -> bool:
"""True iff ``coord`` matches the read-side no-georef placeholder shape.

``coords_from_pixel_geometry`` emits ``np.arange(start, stop,
dtype=np.int64)`` for the y/x coords whenever the source file
carries no GeoTIFF transform tags -- both for full reads
(``start=0``) and windowed reads (``start=window_offset``). See
issues #1710, #1753, #1949.

The writer no longer treats coord shape alone as the no-georef
signal: it checks ``attrs[_NO_GEOREF_KEY]`` instead. See
:func:`_has_no_georef_marker` and issue #2120. This helper remains
available as a diagnostic for the placeholder shape, and the
existing tests in ``test_int_coord_sentinel_2087.py`` still pin
the predicate. It is no longer called from the writer path, so
user-authored int64 step-1 grids that match this pattern but lack
the marker keep their georef on round-trip.
"""
if coord.dtype != np.int64:
return False
n = len(coord)
if n < 1:
attrs = getattr(da, 'attrs', None)
if not isinstance(attrs, Mapping):
return False
return bool(np.array_equal(
coord, np.arange(coord[0], coord[0] + n, dtype=np.int64)
))
return attrs.get(_NO_GEOREF_KEY) is True


def coords_from_pixel_geometry(
Expand Down
26 changes: 18 additions & 8 deletions xrspatial/geotiff/_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -731,19 +731,29 @@ def _check_write_non_uniform_coords(context: Mapping[str, Any]) -> None:
the coords disagree with that step (variable cell size, gaps),
the on-disk file silently misrepresents the geometry.

Exemption: int-dtype coords keep the existing #1969 sentinel
contract. Those coords are the 0..N-1 fallback the no-georef
reader emits, not geographic positions, so non-uniformity is
meaningless for them.
Exemption: arrays carrying the no-georef marker
(``attrs[_NO_GEOREF_KEY] is True``, stamped by the reader on
files without GeoTIFF transform tags) skip the check. Their
coords are the placeholder pixel-index fallback rather than
projected positions, so uniformity is meaningless for them.
Issue #2120 moved this signal off coord dtype: a user-authored
int-coord grid no longer earns the exemption just by having an
integer dtype, only by carrying the marker.

Context keys consumed:

* ``coord_y`` -- ``data.coords['y'].values`` (or equivalent).
* ``coord_x`` -- ``data.coords['x'].values``.
* ``no_georef_marker`` -- ``data.attrs[_NO_GEOREF_KEY] is True``.

Missing or non-numeric coords are out of scope (handled by other
writer validation upstream).
"""
if context.get('no_georef_marker') is True:
# Arrays the reader stamped as no-georef carry the 0..N-1
# placeholder coords. Uniformity is not meaningful there;
# the writer skips transform synthesis entirely.
return
for axis_name in ('y', 'x'):
coord = context.get(f'coord_{axis_name}')
if coord is None:
Expand All @@ -752,10 +762,10 @@ def _check_write_non_uniform_coords(context: Mapping[str, Any]) -> None:
if arr.size < 3:
# Need at least 3 samples to detect non-uniformity.
continue
if np.issubdtype(arr.dtype, np.integer):
# #1969 sentinel exemption.
continue
if not np.issubdtype(arr.dtype, np.floating):
if not (
np.issubdtype(arr.dtype, np.floating)
or np.issubdtype(arr.dtype, np.integer)
):
continue
diffs = np.diff(arr.astype(np.float64))
# Use a relative tolerance pegged to the step magnitude so that
Expand Down
3 changes: 3 additions & 0 deletions xrspatial/geotiff/_writers/eager.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
from .._backends._gpu_helpers import _is_gpu_data
from .._coords import (
_BAND_DIM_NAMES,
_has_no_georef_marker,
coords_to_transform as _coords_to_transform,
require_transform_for_georeferenced as _require_transform_for_georeferenced,
transform_from_attr as _transform_from_attr,
Expand Down Expand Up @@ -304,6 +305,7 @@ def to_geotiff(data: xr.DataArray | np.ndarray,
'attrs_nodatavals': _attrs.get('nodatavals'),
'coord_y': _coord_y,
'coord_x': _coord_x,
'no_georef_marker': _has_no_georef_marker(data),
})

# Up-front validation: catch bad compression names before they reach
Expand Down Expand Up @@ -862,6 +864,7 @@ def _write_vrt_tiled(data, vrt_path, *, crs=None, nodata=None,
'attrs_nodatavals': _attrs.get('nodatavals'),
'coord_y': _coord_y,
'coord_x': _coord_x,
'no_georef_marker': _has_no_georef_marker(data),
})

# Validate compression_level against codec-specific range
Expand Down
2 changes: 2 additions & 0 deletions xrspatial/geotiff/_writers/gpu.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
)
from .._coords import (
_BAND_DIM_NAMES,
_has_no_georef_marker,
coords_to_transform as _coords_to_transform,
require_transform_for_georeferenced as _require_transform_for_georeferenced,
transform_from_attr as _transform_from_attr,
Expand Down Expand Up @@ -324,6 +325,7 @@ def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray | np.ndarray,
'attrs_nodatavals': _attrs.get('nodatavals'),
'coord_y': _coord_y,
'coord_x': _coord_x,
'no_georef_marker': _has_no_georef_marker(data),
})
if max_z_error < 0:
raise ValueError(
Expand Down
71 changes: 62 additions & 9 deletions xrspatial/geotiff/tests/test_int_coord_sentinel_2087.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,58 @@
import xarray as xr

from xrspatial.geotiff import open_geotiff, to_geotiff
from xrspatial.geotiff._coords import _is_no_georef_sentinel
from xrspatial.geotiff._coords import _has_no_georef_marker
from xrspatial.geotiff._geotags import _NO_GEOREF_KEY


# --- Unit checks on the no-georef marker predicate ----------------------
#
# Pre-#2133, ``xrspatial.geotiff._coords`` exported an
# ``_is_no_georef_sentinel`` helper that inspected coord shape (int64,
# ``np.arange``-style). The writer no longer consults that predicate;
# the only signal is ``attrs[_NO_GEOREF_KEY]``. These tests pin the
# marker-based predicate ``_has_no_georef_marker`` that replaced it.


def _arange_int64_shape(coord: np.ndarray) -> bool:
"""Test-local predicate matching the read-side placeholder shape.

``coords_from_pixel_geometry`` emits ``np.arange(start, stop,
dtype=np.int64)`` for the y/x coords whenever the source file
carries no transform tags -- both for full reads (``start=0``) and
windowed reads (``start=window_offset``). This helper exists only
so a few legacy round-trip assertions can verify the on-disk shape
came back unchanged; it is not the production no-georef signal.
"""
if coord.dtype != np.int64:
return False
n = len(coord)
if n < 1:
return False
return bool(np.array_equal(
coord, np.arange(coord[0], coord[0] + n, dtype=np.int64)
))


# --- Unit checks on the sentinel helper itself --------------------------
@pytest.mark.parametrize(
"attrs,expected",
[
({_NO_GEOREF_KEY: True}, True),
({}, False),
({_NO_GEOREF_KEY: False}, False),
({_NO_GEOREF_KEY: 'yes'}, False), # not identity-True
({_NO_GEOREF_KEY: 1}, False), # truthy int, not True
({'other': True}, False),
],
)
def test_marker_predicate_identity_check(attrs, expected):
da = xr.DataArray(
np.zeros((2, 2), dtype=np.float32),
coords={'y': np.arange(2, dtype=np.int64), 'x': np.arange(2, dtype=np.int64)},
dims=('y', 'x'),
attrs=attrs,
)
assert _has_no_georef_marker(da) is expected


@pytest.mark.parametrize(
Expand All @@ -44,8 +92,8 @@
np.array([10, 11, 12], dtype=np.int64),
],
)
def test_sentinel_accepts_arange_int64(coord):
assert _is_no_georef_sentinel(coord)
def test_arange_int64_shape_helper_accepts(coord):
assert _arange_int64_shape(coord)


@pytest.mark.parametrize(
Expand All @@ -59,8 +107,8 @@ def test_sentinel_accepts_arange_int64(coord):
np.array([], dtype=np.int64), # empty
],
)
def test_sentinel_rejects_non_arange(coord):
assert not _is_no_georef_sentinel(coord)
def test_arange_int64_shape_helper_rejects(coord):
assert not _arange_int64_shape(coord)


# --- Round-trip behaviour ------------------------------------------------
Expand Down Expand Up @@ -144,8 +192,13 @@ def test_user_authored_int_grid_with_explicit_transform(tmp_path):

def test_non_uniform_int_coords_raise(tmp_path):
# Non-uniform integer spacing under the old sentinel silently
# stripped georef. Under the tightened sentinel it falls through
# to ``coords_to_transform`` and trips the uniform-spacing check.
# stripped georef. The pre-#2133 fallback caught this via the
# lower-level ``coords_to_transform`` ("not uniformly spaced"
# message). Post-#2133, the write-metadata validator catches it
# first with a different message because the integer-dtype
# exemption has been replaced with a marker-based one. Either
# message satisfies the contract: a non-uniform write must
# raise rather than silently misrepresent the grid.
da = xr.DataArray(
np.zeros((3, 3), dtype=np.float32),
coords={
Expand All @@ -155,7 +208,7 @@ def test_non_uniform_int_coords_raise(tmp_path):
dims=('y', 'x'),
)
path = str(tmp_path / "tmp_2087_non_uniform.tif")
with pytest.raises(ValueError, match="not uniformly spaced"):
with pytest.raises(ValueError, match="non.?uniform"):
to_geotiff(da, path)


Expand Down
Loading
Loading