Hide read_to_array from xrspatial.geotiff public namespace (#1708) (#1711)

* Hide read_to_array from xrspatial.geotiff public namespace (#1708)

The api-consistency sweep on 2026-05-12 flagged an orphan-API leak:
xrspatial/geotiff/__init__.py imports ``read_to_array`` from ``._reader``
so the eager ``open_geotiff`` body can call it, but as a side-effect
that bound the name in the public ``xrspatial.geotiff`` namespace.
``__all__`` does not list it, the module-level "Public API" docstring
does not document it, and the Sphinx pages do not advertise it -- but
``from xrspatial.geotiff import read_to_array`` worked anyway.

The library's own ``test_band_validation_1673.py`` imported it that way
six times, proving the leak was being used as if it were public.
External users who pattern-matched against the test would also be
relying on a name the maintainer never committed to support. A future
cleanup that removed the top-level import would have broken them with
no deprecation path.

This change binds the helper under a leading-underscore name in
``__init__.py``:

    from ._reader import UnsafeURLError
    from ._reader import read_to_array as _read_to_array

and updates each internal call site (six call sites: ``open_geotiff``,
the dask delayed window reader, and four GPU-fallback paths in
``read_geotiff_gpu``). The single test file that imported the leaked
name is updated to import from ``xrspatial.geotiff._reader`` directly,
which is the canonical private path.

``test_namespace_no_leak_1708.py`` pins the namespace contract:
``read_to_array`` must not be an attribute of ``xrspatial.geotiff``,
``__all__`` must not list it, and the canonical
``xrspatial.geotiff._reader.read_to_array`` import must still work.
This guards against a future maintainer re-adding a bare
``from ._reader import read_to_array`` line without realising it would
re-leak the name.

No runtime behaviour change in the supported public API. The deprecated
``plot_geotiff`` continues to be exposed at the top level for backward
compatibility because it has a documented deprecation path; this commit
takes no position on ``plot_geotiff``.

Closes #1708.

* Patch _read_to_array alias in dask astype trace test

The #1708 cleanup hid ``read_to_array`` from ``xrspatial.geotiff`` and
bound the internal alias as ``_read_to_array``. The existing #1624
trace test still set ``xrspatial.geotiff.read_to_array`` via
monkeypatch, which now raises ``AttributeError`` at the setattr call.
Patch the alias the dask worker actually reads instead.

Author: brendancol

xrspatial/geotiff/__init__.py

@@ -43,7 +43,14 @@
     from typing import BinaryIO
 
 from ._geotags import GeoTransform, RASTER_PIXEL_IS_AREA, RASTER_PIXEL_IS_POINT
-from ._reader import UnsafeURLError, read_to_array
+from ._reader import UnsafeURLError
+# ``read_to_array`` is internal: it is used by ``open_geotiff`` and the
+# GPU fallback below but is not in ``__all__`` or the module-level
+# Public API docstring. Bind it under a leading-underscore name so it
+# does not leak into ``xrspatial.geotiff``'s public namespace. Tests
+# and internal callers that genuinely need it can import directly from
+# ``xrspatial.geotiff._reader``. See issue #1708.
+from ._reader import read_to_array as _read_to_array
 from ._writer import write
 
 # All names below are part of the supported public API. ``plot_geotiff``
@@ -807,7 +814,7 @@ def open_geotiff(source, *, dtype=None,
     # in :func:`read_geotiff_dask`. That keeps the two backends in sync
     # on the contract without forcing a second metadata parse here. See
     # issue #1634.
-    arr, geo_info = read_to_array(
+    arr, geo_info = _read_to_array(
         source, window=window,
         overview_level=overview_level, band=band,
         **kwargs,
@@ -2171,9 +2178,9 @@ def _read(http_meta):
             _r2a_kwargs = {}
             if max_pixels is not None:
                 _r2a_kwargs['max_pixels'] = max_pixels
-            arr, _ = read_to_array(source, window=(r0, c0, r1, c1),
-                                   overview_level=overview_level,
-                                   band=band, **_r2a_kwargs)
+            arr, _ = _read_to_array(source, window=(r0, c0, r1, c1),
+                                    overview_level=overview_level,
+                                    band=band, **_r2a_kwargs)
         if nodata is not None:
             # ``arr`` was just decoded by ``_fetch_decode_cog_http_tiles``
             # or ``read_to_array``; both return freshly-allocated buffers
@@ -2677,7 +2684,7 @@ def read_geotiff_gpu(source: str, *,
             # its geo_info and apply our own transform update below so the
             # result is correct regardless of merge order.
             src.close()
-            arr_cpu, _ = read_to_array(
+            arr_cpu, _ = _read_to_array(
                 source, overview_level=overview_level)
             arr_gpu = cupy.asarray(arr_cpu)
             if orientation != 1:
@@ -2843,7 +2850,7 @@ def _read_once():
             # Drop read_to_array's geo_info: orientation transform handling
             # below operates on our pre-extracted geo_info so the 2/3/4 case
             # is covered regardless of #1539's merge state.
-            arr_cpu, _ = read_to_array(
+            arr_cpu, _ = _read_to_array(
                 source, overview_level=overview_level)
             arr_gpu = cupy.asarray(arr_cpu)
             arr_was_cpu_decoded = True
@@ -2856,7 +2863,7 @@ def _read_once():
                     f"({height}, {width}, {samples})"
                 )
     elif has_sparse_tile:
-        arr_cpu, _ = read_to_array(
+        arr_cpu, _ = _read_to_array(
             source, overview_level=overview_level)
         arr_gpu = cupy.asarray(arr_cpu)
         arr_was_cpu_decoded = True
@@ -2913,7 +2920,7 @@ def _read_once():
                 RuntimeWarning,
                 stacklevel=2,
             )
-            arr_cpu, _ = read_to_array(
+            arr_cpu, _ = _read_to_array(
                 source, overview_level=overview_level)
             arr_gpu = cupy.asarray(arr_cpu)
             arr_was_cpu_decoded = True

xrspatial/geotiff/tests/test_band_validation_1673.py

@@ -46,7 +46,7 @@ def multiband_tiff_path(tmp_path):
 
 def test_read_to_array_negative_band_rejected(multiband_tiff_path):
     """``band=-1`` no longer silently selects the last channel."""
-    from xrspatial.geotiff import read_to_array
+    from xrspatial.geotiff._reader import read_to_array
 
     path, _ = multiband_tiff_path
     with pytest.raises(IndexError, match="band=-1 out of range"):
@@ -55,7 +55,7 @@ def test_read_to_array_negative_band_rejected(multiband_tiff_path):
 
 def test_read_to_array_band_equal_to_samples_rejected(multiband_tiff_path):
     """``band=samples_per_pixel`` (off-by-one) raises a typed error."""
-    from xrspatial.geotiff import read_to_array
+    from xrspatial.geotiff._reader import read_to_array
 
     path, _ = multiband_tiff_path
     # File has 3 bands; valid indices are 0, 1, 2.
@@ -65,7 +65,7 @@ def test_read_to_array_band_equal_to_samples_rejected(multiband_tiff_path):
 
 def test_read_to_array_band_far_above_samples_rejected(multiband_tiff_path):
     """A wildly out-of-range band index gives the same typed error."""
-    from xrspatial.geotiff import read_to_array
+    from xrspatial.geotiff._reader import read_to_array
 
     path, _ = multiband_tiff_path
     with pytest.raises(IndexError, match="band=103 out of range"):
@@ -74,7 +74,7 @@ def test_read_to_array_band_far_above_samples_rejected(multiband_tiff_path):
 
 def test_read_to_array_valid_band_still_works(multiband_tiff_path):
     """Valid band indices keep working after the validation guard."""
-    from xrspatial.geotiff import read_to_array
+    from xrspatial.geotiff._reader import read_to_array
 
     path, arr = multiband_tiff_path
     out, _ = read_to_array(path, band=1)
@@ -83,7 +83,7 @@ def test_read_to_array_valid_band_still_works(multiband_tiff_path):
 
 def test_read_to_array_band_none_still_returns_all_bands(multiband_tiff_path):
     """``band=None`` still returns the full multi-band array."""
-    from xrspatial.geotiff import read_to_array
+    from xrspatial.geotiff._reader import read_to_array
 
     path, arr = multiband_tiff_path
     out, _ = read_to_array(path)
@@ -92,7 +92,8 @@ def test_read_to_array_band_none_still_returns_all_bands(multiband_tiff_path):
 
 def test_backend_parity_negative_band(multiband_tiff_path):
     """Local eager and dask paths raise the same error for ``band=-1``."""
-    from xrspatial.geotiff import read_geotiff_dask, read_to_array
+    from xrspatial.geotiff import read_geotiff_dask
+    from xrspatial.geotiff._reader import read_to_array
 
     path, _ = multiband_tiff_path
 
@@ -110,7 +111,8 @@ def test_backend_parity_negative_band(multiband_tiff_path):
 
 def test_backend_parity_band_equal_to_samples(multiband_tiff_path):
     """Local eager and dask paths agree on the off-by-one rejection."""
-    from xrspatial.geotiff import read_geotiff_dask, read_to_array
+    from xrspatial.geotiff import read_geotiff_dask
+    from xrspatial.geotiff._reader import read_to_array
 
     path, _ = multiband_tiff_path
 

xrspatial/geotiff/tests/test_dask_no_op_astype_1624.py

@@ -104,7 +104,12 @@ def wrapped_r2a(*args, **kwargs):
         captured.append(tracked)
         return tracked, meta
 
-    monkeypatch.setattr(gt, 'read_to_array', wrapped_r2a)
+    # ``read_geotiff_dask``'s per-chunk worker calls the alias
+    # ``_read_to_array`` bound in ``xrspatial.geotiff``. Patch that
+    # binding; patching ``_reader.read_to_array`` would not affect the
+    # already-imported alias. See issue #1708 for why ``read_to_array``
+    # is internal.
+    monkeypatch.setattr(gt, '_read_to_array', wrapped_r2a)
 
     dk = read_geotiff_dask(path, chunks=4)
     dk.compute()

xrspatial/geotiff/tests/test_namespace_no_leak_1708.py

@@ -0,0 +1,64 @@
+"""Regression test for #1708: ``read_to_array`` no longer leaks into
+``xrspatial.geotiff``'s public namespace.
+
+Before the fix, ``xrspatial/geotiff/__init__.py`` did
+
+    from ._reader import UnsafeURLError, read_to_array
+
+so ``from xrspatial.geotiff import read_to_array`` worked even though
+``read_to_array`` was not in ``__all__`` and not documented as public.
+The api-consistency sweep on 2026-05-12 flagged this as an orphan API
+(Cat 5): the name is reachable from the public namespace and tests
+relied on it, but the maintainer had not committed to it as a
+supported entry point. A future cleanup that removed the top-level
+import would have broken users with no deprecation path.
+
+The fix binds the helper under a leading-underscore name in
+``__init__.py`` (``_read_to_array``) and updates the single internal
+test that depended on the top-level import to go through
+``xrspatial.geotiff._reader`` directly.
+
+This module pins the public-namespace contract so a future maintainer
+who re-adds a bare ``from ._reader import read_to_array`` to
+``__init__.py`` fails CI before merging.
+"""
+from __future__ import annotations
+
+import pytest
+
+import xrspatial.geotiff as g
+
+
+def test_read_to_array_not_in_module_namespace():
+    """``read_to_array`` is internal and must not be a public attribute
+    of ``xrspatial.geotiff``. Tests and internal callers go through
+    ``xrspatial.geotiff._reader`` directly."""
+    assert not hasattr(g, 'read_to_array'), (
+        "read_to_array leaked into xrspatial.geotiff's public namespace. "
+        "It should be bound as _read_to_array (underscore-prefix) so it "
+        "does not appear in `from xrspatial.geotiff import *` or tab "
+        "completion. See issue #1708."
+    )
+
+
+def test_read_to_array_not_in_all():
+    """``__all__`` does not advertise ``read_to_array``. (Pre-existing
+    state, pinned so a future change can't quietly promote it without
+    also adding it to the module-level Public API docstring.)"""
+    assert 'read_to_array' not in g.__all__
+
+
+def test_read_to_array_still_importable_from_reader_submodule():
+    """The function itself is still reachable via the canonical private
+    path so tests and internal code keep working."""
+    from xrspatial.geotiff._reader import read_to_array
+
+    assert callable(read_to_array)
+
+
+def test_import_from_top_level_now_fails():
+    """Direct top-level import is the regression we are guarding
+    against. Before #1708 this succeeded; afterwards it must raise
+    ImportError."""
+    with pytest.raises(ImportError):
+        from xrspatial.geotiff import read_to_array  # noqa: F401