Route sidecar georef tags through the sidecar's bytes (#2315)

The local eager reader and the metadata-only path both swap pixel
bytes over to the sidecar when the selected IFD lives there, but the
georef extraction call right below the swap still passed the base
file's data + header.byte_order. This worked under the usual GDAL
convention -- sidecar IFDs carry no geokeys -- but silently broke when
a sidecar declared its own GeoKeyDirectory or model tags.

extract_geo_info_with_overview_inheritance now accepts an optional
sidecar_origin mapping from id(ifd) to (data, byte_order). When the
selected IFD is in the mapping AND declares one of the georef-bearing
tags (33550, 33922, 34264, 34735), the helper resolves tag offsets
against the sidecar's bytes. Sidecars without geokeys still inherit
from the base file's level-0 IFD as before.

Both call sites (_reader.py and __init__.py:_read_geo_info) now build
and forward the mapping.

Includes a synthetic regression test covering both cases: sidecar
with its own ModelPixelScale / ModelTiepoint / GeoKeyDirectory, and
sidecar with those tags stripped (today's GDAL-convention behavior).

Author: brendancol

xrspatial/geotiff/__init__.py

@@ -273,24 +273,38 @@ def _read_geo_info(source, *, overview_level: int | None = None,
         # so ``overview_level`` indexes both internal and external
         # overviews (issue #2112). Local file paths only.
         from ._sidecar import attach_sidecar_origin, find_sidecar, load_sidecar
+        sidecar_origin: dict[int, tuple] = {}
         sidecar_path = find_sidecar(source)
         if sidecar_path is not None:
             sidecar = load_sidecar(sidecar_path)
-            # Metadata-only path: drop the origin mapping. The reader
-            # only needs the merged IFD list to resolve the requested
-            # ``overview_level`` against; strip/tile bytes are sliced by
-            # ``read_to_array`` on the actual read.
-            attach_sidecar_origin(
+            # The origin mapping is consumed below for georef extraction
+            # only -- strip/tile bytes are sliced by ``read_to_array`` on
+            # the actual read. A sidecar IFD that carries its own
+            # GeoKeyDirectory / ModelPixelScale / ModelTiepoint /
+            # ModelTransformation needs the sidecar's byte order to
+            # parse cleanly; without the mapping the helper falls back
+            # to the base file's bytes (today's default, correct under
+            # the usual GDAL convention). See issue #2315.
+            sidecar_origin = attach_sidecar_origin(
                 sidecar.ifds, sidecar.data, sidecar.header)
             ifds = ifds + sidecar.ifds
         ifd = select_overview_ifd(ifds, overview_level)
         # Inherit georef from the level-0 IFD when the overview itself
         # has no geokeys (issue #1640). Pass-through for level 0. The
         # sidecar IFDs typically lack geokeys so the inheritance pulls
-        # from the base file's full-resolution IFD as GDAL does.
+        # from the base file's full-resolution IFD as GDAL does. When a
+        # sidecar IFD does declare its own georef payload, ``georef_origin``
+        # routes the parse to the sidecar's bytes / byte order so the
+        # sidecar's georef wins. See issue #2315.
+        georef_origin = (
+            {iid: (od, oh.byte_order)
+             for iid, (od, oh) in sidecar_origin.items()}
+            if sidecar_origin else None
+        )
         geo_info = extract_geo_info_with_overview_inheritance(
             ifd, ifds, data, header.byte_order,
-            allow_rotated=allow_rotated)
+            allow_rotated=allow_rotated,
+            sidecar_origin=georef_origin)
         bps = resolve_bits_per_sample(ifd.bits_per_sample)
         file_dtype = tiff_dtype_to_numpy(bps, ifd.sample_format)
         _validate_predictor_sample_format(ifd.predictor, ifd.sample_format)

xrspatial/geotiff/_geotags.py

@@ -1085,13 +1085,43 @@ def extract_geo_info(ifd: IFD, data: bytes | memoryview,
     )
 
 
+# Tag IDs that carry georeferencing payload (offsets into either the
+# IFD entry's inline value or the file's byte stream). When a sidecar
+# IFD declares any of these, the georef extractor must parse against
+# the sidecar's bytes / byte order, not the base file's. See issue
+# #2315.
+_GEOREF_PAYLOAD_TAGS = frozenset({
+    33550,  # TAG_MODEL_PIXEL_SCALE
+    33922,  # TAG_MODEL_TIEPOINT
+    34264,  # TAG_MODEL_TRANSFORMATION
+    34735,  # TAG_GEO_KEY_DIRECTORY
+})
+
+
+def _ifd_has_georef_payload(ifd: IFD) -> bool:
+    """Return True if ``ifd`` declares any georef-bearing tag.
+
+    The GDAL convention is that an external ``.tif.ovr`` sidecar
+    inherits georef from the base file -- the sidecar IFDs do not
+    re-declare ModelPixelScale / ModelTiepoint / GeoKeyDirectory.
+    Hand-rolled sidecars and non-GDAL writers can break that convention,
+    and when they do the sidecar's georef tags must be parsed against
+    the sidecar's byte buffer rather than the base file's. This helper
+    is the conservative gate that flips the data / byte_order swap in
+    :func:`extract_geo_info_with_overview_inheritance`. See issue #2315.
+    """
+    entries = ifd.entries
+    return any(tag in entries for tag in _GEOREF_PAYLOAD_TAGS)
+
+
 def extract_geo_info_with_overview_inheritance(
     ifd: IFD,
     ifds: list,
     data: bytes | memoryview,
     byte_order: str,
     *,
     allow_rotated: bool = False,
+    sidecar_origin: dict | None = None,
 ) -> GeoInfo:
     """Extract geo metadata, inheriting from level 0 when the IFD lacks it.
 
@@ -1141,12 +1171,36 @@ def extract_geo_info_with_overview_inheritance(
         Full file bytes (forwarded to ``extract_geo_info``).
     byte_order : str
         ``'<'`` or ``'>'`` (forwarded to ``extract_geo_info``).
+    sidecar_origin : dict, optional
+        Mapping from ``id(ifd)`` to ``(data, byte_order)`` for IFDs
+        that live in an external ``.tif.ovr`` sidecar. When the
+        selected IFD is from a sidecar that declares its own georef
+        payload (ModelPixelScale / ModelTiepoint /
+        ModelTransformation / GeoKeyDirectory), this mapping tells
+        the extractor which byte buffer the tag offsets resolve
+        against. IFDs absent from the mapping fall through to
+        ``(data, byte_order)`` for the base file. The conservative
+        default is ``None``, which preserves the legacy behavior
+        (read sidecar IFDs against the base file's bytes -- correct
+        only when the sidecar has no geokeys, which is the GDAL
+        convention). See issue #2315.
 
     Returns
     -------
     GeoInfo
     """
-    info = extract_geo_info(ifd, data, byte_order,
+    # When the selected IFD lives in a sidecar AND it carries its own
+    # georef payload, resolve tag offsets against the sidecar's bytes /
+    # byte order. The sidecar-without-geokeys convention (GDAL) keeps
+    # ``(data, byte_order)`` from the base file and inherits below.
+    # See issue #2315.
+    sel_data, sel_byte_order = data, byte_order
+    if sidecar_origin is not None:
+        origin = sidecar_origin.get(id(ifd))
+        if origin is not None and _ifd_has_georef_payload(ifd):
+            sel_data, sel_byte_order = origin
+
+    info = extract_geo_info(ifd, sel_data, sel_byte_order,
                             allow_rotated=allow_rotated)
 
     # Overview IFDs have NewSubfileType bit 0 set; mask IFDs (bit 2) and

xrspatial/geotiff/_reader.py

@@ -227,13 +227,24 @@ def _read_to_array(source, *, window=None, overview_level: int | None = None,
         # Inherit georef from level 0 when an overview IFD lacks its own
         # geokeys (issue #1640). For overview_level=0 (or None) this is a
         # no-op: the helper short-circuits when the IFD is not a
-        # NewSubfileType=overview entry. Sidecar IFDs always lack
-        # geokeys, so the inheritance pulls from the base file's
-        # level-0 IFD (kept first in the merged list) which is the
-        # GDAL convention.
+        # NewSubfileType=overview entry. Sidecar IFDs typically lack
+        # geokeys (the GDAL convention), so the inheritance pulls from
+        # the base file's level-0 IFD (kept first in the merged list).
+        # A sidecar that does declare its own georef payload is a corner
+        # case: ``georef_origin`` maps the sidecar IFDs to
+        # ``(data, byte_order)`` from the sidecar so the helper resolves
+        # those tags against the right buffer. Sidecar IFDs without
+        # geokeys still inherit from the base file via the existing
+        # overview-inheritance path. See issues #1640 and #2315.
+        georef_origin = (
+            {iid: (od, oh.byte_order)
+             for iid, (od, oh) in sidecar_origin.items()}
+            if sidecar_origin else None
+        )
         geo_info = extract_geo_info_with_overview_inheritance(
             ifd, ifds, data, header.byte_order,
-            allow_rotated=allow_rotated)
+            allow_rotated=allow_rotated,
+            sidecar_origin=georef_origin)
 
         # Orientation tag (274): values 2-8 mean the stored pixel order
         # differs from display order. We need to remap the array post