Skip to content

Reject a zero or non-finite ModelPixelScale on GeoTIFF read (#3331)#3332

Merged
brendancol merged 3 commits into
mainfrom
deep-sweep-accuracy-geotiff-2026-06-14-01
Jun 15, 2026
Merged

Reject a zero or non-finite ModelPixelScale on GeoTIFF read (#3331)#3332
brendancol merged 3 commits into
mainfrom
deep-sweep-accuracy-geotiff-2026-06-14-01

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #3331.

_geotags._extract_transform read sx = scale[0] / sy = scale[1] (and the ModelTransformation diagonal m[0] / m[5]) with no finite-nonzero check, then coords_from_pixel_geometry built arange(N) * pixel_width + origin. A zero pixel size collapsed the axis onto the origin; a NaN / Inf one filled it with NaN / Inf. The read returned a degenerate raster with no error.

The VRT read path already raises VRTUnsupportedError for a zero res_x / res_y, and the writer raises NonUniformCoordsError for a zero-step coord axis. This adds the matching rejection on the direct-TIFF read path.

  • New DegeneratePixelSizeError (subclass of GeoTIFFAmbiguousMetadataError, so except ValueError keeps catching it), exported from xrspatial.geotiff and documented in the safe-IO error table.
  • A _check_finite_nonzero_pixel_size guard in _extract_transform covering the scale-only, ModelTiepoint + ModelPixelScale, and ModelTransformation paths. The unit-scale tiepoint-only fallback (always 1.0) is untouched, and the rotated allow_rotated path (which returns no-georef) is untouched.

Backend coverage: the guard sits in the shared geo-extraction path, so numpy, dask, gpu, and dask+gpu all reject the file identically. Verified all four locally on a degenerate fixture (CUDA available on this host).

Test plan:

  • test_degenerate_pixel_size_3331.py: zero / NaN / Inf on all three extraction paths, the subclass contract, positive controls, and an end-to-end open_geotiff rejection.
  • Existing geotags / coords / georef / exception-export suites pass (354 + the broader unit + read suites: 2271 passed, 5 skipped).
  • flake8 + isort clean on the touched files.

The direct-TIFF read path built coordinate arrays as
arange(N) * pixel_width + origin straight from the ModelPixelScale (or
ModelTransformation diagonal) with no finite-nonzero check. A zero pixel
size collapsed the whole axis onto the origin; a NaN / Inf one filled it
with NaN / Inf. Either way the read returned a degenerate raster with no
error, so a downstream spatial op ran on coordinates that did not
describe the data.

The VRT read path already rejected a zero res_x / res_y
(VRTUnsupportedError) and the writer rejected a zero-step coord axis
(NonUniformCoordsError); this brings the direct-TIFF read path in line.

Add DegeneratePixelSizeError (a GeoTIFFAmbiguousMetadataError subclass)
and a _check_finite_nonzero_pixel_size guard in _extract_transform,
covering the scale-only, ModelTiepoint + ModelPixelScale, and
ModelTransformation paths. The guard sits in the shared geo-extraction
path, so all four backends (numpy / dask / gpu / dask+gpu) reject the
file identically.

Closes #3331
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label Jun 15, 2026

@brendancol brendancol left a comment

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.

PR Review: Reject a zero or non-finite ModelPixelScale on GeoTIFF read

Read all five changed files in full on the head branch.

Blockers

None.

Suggestions

None.

Nits

  • _geotags.py _check_finite_nonzero_pixel_size: for the ModelPixelScale path the caller passes sx, sy, but the stored pixel_height is -sy, so the message names pixel_height while the tag value is sy. For the zero case the sign matches, and for NaN / Inf the sign is immaterial, so the message is still accurate. Not worth changing.

What looks good

  • The guard lives in the single shared _extract_transform, so every backend (numpy / dask / gpu / dask+gpu) rejects the file identically. Confirmed by reading the call graph and running all four locally against a degenerate fixture.
  • All three georeferenced return paths are covered (scale-only, tiepoint + scale, ModelTransformation diagonal). The unit-scale tiepoint fallback (literal 1.0) and the allow_rotated no-georef path are left alone, which is correct.
  • A NaN ModelTransformation diagonal still ends up rejected: the rotation-tolerance check short-circuits on NaN (x > NaN is False) and falls through to the new guard. The nan_width / nan_height transformation cases pin this.
  • DegeneratePixelSizeError subclasses GeoTIFFAmbiguousMetadataError (and therefore ValueError), so existing except ValueError callers keep catching it. Exported from the package and added to the safe-IO error table next to its siblings.
  • Tests cover zero / NaN / +-Inf on all three paths, the subclass contract, a positive control, and an end-to-end open_geotiff rejection.

Checklist

  • Behaviour matches the sibling guards (VRT VRTUnsupportedError, writer NonUniformCoordsError)
  • All four backends reject consistently (shared extraction path)
  • NaN / Inf handling correct, including the NaN-diagonal fall-through
  • Edge cases covered (zero, NaN, +-Inf x both axes x three tag paths)
  • No premature materialization (validation runs at tag-parse time, before pixel decode)
  • Benchmark not needed (validation guard, not a compute path)
  • Docs updated (safe-IO error table)
  • Docstrings present on the new error class and helper

The new error is exported from xrspatial.geotiff.__all__, so the frozen
expected-name set in test_all_lists_supported_functions must list it too,
or the gate fails on the set-equality assertion.
@brendancol brendancol merged commit abf1de3 into main Jun 15, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GeoTIFF read accepts a zero or non-finite ModelPixelScale and returns degenerate coords

1 participant