Commit 2ba24b1
authored
* geotiff: bundle remaining ambiguous-metadata fail-closed checks (#1987)
Bundles four of the five remaining #1987 slices (PRs 2, 3, 4, 7 in the
issue's numbering). PR 6 (``ConflictingCRSError``) landed earlier as
this branch's parent. PR 5 (``MixedBandMetadataError``) is intentionally
deferred to a follow-up that also migrates ~35 existing VRT test
fixtures via the new ``band_nodata='first'`` opt-out kwarg.
Active checks added in this PR
------------------------------
* ``UnparseableCRSError`` (#1987 PR 2):
- Write: typed the existing ``_validate_crs_fallback`` raise from
plain ``ValueError`` to ``UnparseableCRSError``. No behaviour
change (subclass relationship).
- Read: new ``_check_read_unparseable_crs`` that runs ``pyproj.CRS.
from_user_input`` on ``crs_wkt`` and raises if pyproj cannot parse.
Tolerates pyproj-parseable placeholders like ``"EPSG:4326"`` that
the GDAL VRT ``<SRS>`` convention stashes into ``crs_wkt``.
- Opt-out: ``allow_unparseable_crs=True`` kwarg, threaded through
``open_geotiff`` / ``read_geotiff_dask`` / ``read_geotiff_gpu`` /
``read_vrt``.
* ``RotatedTransformError`` (#1987 PR 3):
- Read: new ``_check_read_rotated_transform`` that rejects affine
transforms with non-zero b / d terms (rasterio Affine ordering).
Downstream xrspatial ops (slope, aspect, hillshade, proximity,
zonal) assume axis-aligned grids; a rotated transform silently
produced wrong results.
- Opt-out: ``allow_rotated=True`` kwarg, same threading as
``allow_unparseable_crs``.
* ``NonUniformCoordsError`` (#1987 PR 4):
- Write: new ``_check_write_non_uniform_coords`` that diffs the
``y`` and ``x`` coord arrays against the first step and rejects
when relative drift exceeds 1e-5 (mirrors the existing #1720
coord-regularity tolerance). The int-dtype sentinel from #1969 is
exempted (the no-georef fallback uses 0..N-1 ints which the writer
treats specially).
- No new kwarg: the fix is to resample, not to opt out.
* ``ConflictingNodataError`` (#1987 PR 7):
- Write: new ``_check_write_conflicting_nodata`` that refuses when
``attrs['nodata']`` disagrees with every concrete entry in
``attrs['nodatavals']``. ``None`` and NaN entries in the rioxarray
tuple are skipped (same convention as ``_resolve_nodata_attr``).
NaN as the canonical value paired with a concrete numeric in the
tuple also raises -- "NaN is the sentinel" and "X is the sentinel"
contradict.
- Opt-out: explicit ``nodata=`` writer kwarg overrides both attrs
and bypasses the check.
Shared infrastructure
---------------------
* ``_attrs.py`` gains ``_validate_read_geo_info(geo_info, *, window,
allow_rotated, allow_unparseable_crs)`` -- one helper called from
the four read backends so the check site is uniform.
* Each writer entry point (``to_geotiff``, ``_write_vrt_tiled``,
``write_geotiff_gpu``) now builds a context dict carrying
``crs_kwarg`` / ``attrs_crs`` / ``attrs_crs_wkt`` /
``nodata_kwarg`` / ``attrs_nodata`` / ``attrs_nodatavals`` /
``coord_y`` / ``coord_x`` and feeds it to ``validate_write_metadata``.
Deferred: MixedBandMetadataError (#1987 PR 5)
---------------------------------------------
The mixed-band check function and its constants are defined in
``_validation.py`` but the registration call is commented out. The
``band_nodata=`` kwarg is threaded through both ``read_vrt`` and
``_read_vrt_chunked`` so the follow-up PR is a one-line registration
plus the test-fixture migration. About 35 existing VRT tests would
need to opt in to ``band_nodata='first'`` to keep their legacy
assertions, which is its own commit.
Test updates
------------
* ``test_attrs_contract_aliases_1984.py::test_canonical_nodata_wins_over_aliases``
split into a resolver-layer test (still asserts canonical wins) and a
write-layer test (now asserts ``ConflictingNodataError``).
* ``test_nodata_attr_aliases_1582.py::test_explicit_nodata_attr_wins_over_aliases``
updated to expect ``ConflictingNodataError`` on the disagreement and to
show the ``nodata=`` kwarg opt-out.
* ``test_reader_kwarg_order_1935.py`` updated: the new
``allow_rotated`` / ``allow_unparseable_crs`` kwargs join the
canonical order; ``band_nodata`` goes in ``read_vrt``'s
``allowed_tail`` since it is VRT-specific; ``read_geotiff_gpu``'s
deprecated ``gpu`` alias moved back to the tail.
* ``test_ambiguous_metadata_hooks_1987.py`` framework tests now use an
opaque ``_dispatch_probe`` payload key instead of ``"crs_wkt":
"EPSG:4326"`` / ``"MALFORMED"`` placeholders that the newly registered
``_check_read_unparseable_crs`` would refuse.
* ``test_remaining_fail_closed_1987.py`` -- 19 new tests covering each
active check plus the round-trip read-write contract.
Verification
------------
- ``pytest xrspatial/geotiff/tests/ -k 'not gpu and not cuda'`` --
3013 passed (was 2994 pre-PR, +19 new).
- ``pytest xrspatial/geotiff/tests/test_remaining_fail_closed_1987.py
-v`` -- 19 passed.
Refs #1987.
* geotiff: address review suggestions and nits on #1987 fail-closed bundle
- vrt.py: eager read_vrt now parses the VRT XML and runs the #1987
read-side validators *before* _read_vrt_internal materialises the
mosaic, so a rejected file fails fast instead of loading the full
array into host memory first. The parsed VRTDataset is threaded
through via the existing `parsed=` kwarg so the XML is parsed only
once.
- _validation.py: extracted `_gdal_geotransform_to_affine_tuple` so
the eager and chunked VRT call sites share one GDAL->rasterio
reorder helper instead of duplicating the index shuffle.
- _attrs.py: documented in `_validate_read_geo_info` that the
built transform is always axis-aligned (rotated TIFFs raise
NotImplementedError upstream in `_geotags`), so the rotated check
fires only on the VRT path.
- tests: added direct coverage for the row-axis rotation branch
(GDAL GT[4] non-zero) and for the zero-step / constant-float-coord
branch in `_check_write_non_uniform_coords`. Refactored the
rotated-VRT fixture to take a `geo_transform` kwarg so both
rotation axes share the same builder.
1 parent 8bc981d commit 2ba24b1
15 files changed
Lines changed: 1097 additions & 36 deletions
File tree
- xrspatial/geotiff
- _backends
- _writers
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
16 | 20 | | |
17 | 21 | | |
18 | 22 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
79 | 79 | | |
80 | 80 | | |
81 | 81 | | |
| 82 | + | |
82 | 83 | | |
83 | 84 | | |
84 | 85 | | |
| |||
251 | 252 | | |
252 | 253 | | |
253 | 254 | | |
| 255 | + | |
| 256 | + | |
254 | 257 | | |
255 | 258 | | |
256 | 259 | | |
| |||
441 | 444 | | |
442 | 445 | | |
443 | 446 | | |
444 | | - | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
445 | 451 | | |
446 | 452 | | |
447 | 453 | | |
| |||
466 | 472 | | |
467 | 473 | | |
468 | 474 | | |
| 475 | + | |
| 476 | + | |
469 | 477 | | |
470 | 478 | | |
471 | 479 | | |
472 | 480 | | |
473 | 481 | | |
474 | 482 | | |
475 | 483 | | |
476 | | - | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
477 | 487 | | |
478 | 488 | | |
479 | 489 | | |
| |||
514 | 524 | | |
515 | 525 | | |
516 | 526 | | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
517 | 535 | | |
518 | 536 | | |
519 | 537 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
467 | 467 | | |
468 | 468 | | |
469 | 469 | | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
470 | 521 | | |
471 | 522 | | |
472 | 523 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | | - | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
21 | 25 | | |
22 | 26 | | |
23 | 27 | | |
| |||
34 | 38 | | |
35 | 39 | | |
36 | 40 | | |
37 | | - | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
38 | 44 | | |
39 | 45 | | |
40 | 46 | | |
| |||
294 | 300 | | |
295 | 301 | | |
296 | 302 | | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
297 | 310 | | |
298 | 311 | | |
299 | 312 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
23 | | - | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
24 | 28 | | |
25 | 29 | | |
26 | 30 | | |
| |||
80 | 84 | | |
81 | 85 | | |
82 | 86 | | |
| 87 | + | |
| 88 | + | |
83 | 89 | | |
84 | 90 | | |
85 | 91 | | |
| |||
233 | 239 | | |
234 | 240 | | |
235 | 241 | | |
| 242 | + | |
| 243 | + | |
236 | 244 | | |
237 | 245 | | |
238 | 246 | | |
| |||
382 | 390 | | |
383 | 391 | | |
384 | 392 | | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
385 | 398 | | |
386 | 399 | | |
387 | 400 | | |
| |||
710 | 723 | | |
711 | 724 | | |
712 | 725 | | |
| 726 | + | |
| 727 | + | |
| 728 | + | |
| 729 | + | |
| 730 | + | |
| 731 | + | |
713 | 732 | | |
714 | 733 | | |
715 | 734 | | |
| |||
881 | 900 | | |
882 | 901 | | |
883 | 902 | | |
884 | | - | |
| 903 | + | |
| 904 | + | |
| 905 | + | |
885 | 906 | | |
886 | 907 | | |
887 | 908 | | |
| |||
941 | 962 | | |
942 | 963 | | |
943 | 964 | | |
| 965 | + | |
| 966 | + | |
944 | 967 | | |
945 | 968 | | |
946 | 969 | | |
| |||
952 | 975 | | |
953 | 976 | | |
954 | 977 | | |
| 978 | + | |
| 979 | + | |
955 | 980 | | |
956 | 981 | | |
957 | 982 | | |
| |||
973 | 998 | | |
974 | 999 | | |
975 | 1000 | | |
976 | | - | |
| 1001 | + | |
| 1002 | + | |
| 1003 | + | |
977 | 1004 | | |
978 | 1005 | | |
979 | 1006 | | |
| |||
1159 | 1186 | | |
1160 | 1187 | | |
1161 | 1188 | | |
| 1189 | + | |
| 1190 | + | |
| 1191 | + | |
| 1192 | + | |
| 1193 | + | |
| 1194 | + | |
1162 | 1195 | | |
1163 | 1196 | | |
1164 | 1197 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
37 | 37 | | |
38 | 38 | | |
39 | 39 | | |
40 | | - | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
41 | 44 | | |
42 | 45 | | |
43 | 46 | | |
| |||
158 | 161 | | |
159 | 162 | | |
160 | 163 | | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
161 | 167 | | |
162 | 168 | | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
163 | 198 | | |
164 | 199 | | |
165 | | - | |
| 200 | + | |
166 | 201 | | |
167 | 202 | | |
168 | 203 | | |
| |||
339 | 374 | | |
340 | 375 | | |
341 | 376 | | |
342 | | - | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
343 | 381 | | |
344 | 382 | | |
345 | 383 | | |
| |||
383 | 421 | | |
384 | 422 | | |
385 | 423 | | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
386 | 442 | | |
387 | 443 | | |
388 | 444 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
19 | 20 | | |
20 | 21 | | |
| |||
165 | 166 | | |
166 | 167 | | |
167 | 168 | | |
168 | | - | |
| 169 | + | |
169 | 170 | | |
170 | 171 | | |
171 | 172 | | |
172 | 173 | | |
173 | 174 | | |
174 | 175 | | |
175 | 176 | | |
176 | | - | |
| 177 | + | |
177 | 178 | | |
178 | 179 | | |
179 | 180 | | |
| |||
0 commit comments