Skip to content

Commit 9d8a3e3

Browse files
committed
Address review nits: base_ifd sidecar lookup + test invariant (#2315)
* `_geotags.py`: when a base IFD is also tracked in `sidecar_origin` (uncommon: a file with no full-resolution IFD on the base side), resolve its georef against the sidecar's bytes too. Mirrors the lookup applied to the selected IFD. * `test_sidecar_own_geokeys_2315.py`: assert the first existing IFD tag id is > 254 before inserting the NewSubfileType entry at position 0, so a future writer change cannot silently break the fixture's "insert at front" assumption. * `_cog_http.py`: leave a cross-reference comment noting that the `sidecar_origin` routing added in #2315 is not threaded through the HTTP / fsspec metadata path yet; that gap is tracked by the separate HTTP sidecar-byte-order finding.
1 parent 1924122 commit 9d8a3e3

3 files changed

Lines changed: 31 additions & 2 deletions

File tree

xrspatial/geotiff/_cog_http.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,13 @@ def _parse_cog_http_meta(
239239
# IFD lives in the sidecar; that mirrors the eager local reader,
240240
# whose sidecar IFDs typically carry no out-of-line geokeys and
241241
# inherit from level-0 (which sits in the base buffer). #2239.
242+
#
243+
# The ``sidecar_origin`` kwarg added in #2315 for the eager local /
244+
# fsspec paths is intentionally not threaded here. A separate fix
245+
# is tracked in the HTTP / dask sidecar-byte-order finding (see
246+
# the linked issue / PR for the HTTP side). When that lands, this
247+
# call should pick up the same mapping so an HTTP sidecar with
248+
# its own geokeys is parsed against the sidecar bytes too.
242249
geo_info = extract_geo_info_with_overview_inheritance(
243250
ifd, ifds, header_bytes, header.byte_order,
244251
allow_rotated=allow_rotated)

xrspatial/geotiff/_geotags.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1225,7 +1225,19 @@ def extract_geo_info_with_overview_inheritance(
12251225
if base_ifd is None:
12261226
return info
12271227

1228-
base_info = extract_geo_info(base_ifd, data, byte_order,
1228+
# Mirror the sidecar-origin routing for ``base_ifd``. The base IFD
1229+
# normally lives in the base file (``data`` / ``byte_order`` are
1230+
# correct), but a file with no full-resolution IFD of its own could
1231+
# land here with ``base_ifd`` resolved out of a sidecar. The lookup
1232+
# is the same shape as the one applied to ``ifd`` above. See
1233+
# review nit on #2315.
1234+
base_data, base_byte_order = data, byte_order
1235+
if sidecar_origin is not None:
1236+
base_origin = sidecar_origin.get(id(base_ifd))
1237+
if base_origin is not None:
1238+
base_data, base_byte_order = base_origin
1239+
1240+
base_info = extract_geo_info(base_ifd, base_data, base_byte_order,
12291241
allow_rotated=allow_rotated)
12301242

12311243
# Inherit the per-IFD metadata that the COG writer emits only on the

xrspatial/geotiff/tests/test_sidecar_own_geokeys_2315.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,17 @@ def _mark_first_ifd_as_overview(path):
106106

107107
# The IFD entries are sorted by tag id. 254 sorts before every tag
108108
# ``to_geotiff`` emits in practice (the smallest the writer uses is
109-
# 256 / ImageWidth), so the new entry goes at position 0.
109+
# 256 / ImageWidth), so the new entry goes at position 0. Assert
110+
# the invariant so a future writer change that emits a tag <= 254
111+
# fails this fixture loudly instead of silently producing an
112+
# out-of-order IFD that the reader could later reject. (Review nit
113+
# on #2315.)
114+
if n_entries > 0:
115+
first_tag = struct.unpack_from("<H", raw, first_ifd_offset + 2)[0]
116+
assert first_tag > NSF, (
117+
f"test fixture invariant: first emitted tag must be > {NSF}, "
118+
f"got {first_tag}"
119+
)
110120
insert_pos = first_ifd_offset + 2
111121

112122
next_ifd_off_pos = first_ifd_offset + 2 + n_entries * 12

0 commit comments

Comments
 (0)