diff --git a/xrspatial/geotiff/__init__.py b/xrspatial/geotiff/__init__.py index 70f35984..ecab2580 100644 --- a/xrspatial/geotiff/__init__.py +++ b/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`` @@ -683,7 +690,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, @@ -2026,9 +2033,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 @@ -2530,7 +2537,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: @@ -2696,7 +2703,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 @@ -2709,7 +2716,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 @@ -2766,7 +2773,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 diff --git a/xrspatial/geotiff/tests/test_band_validation_1673.py b/xrspatial/geotiff/tests/test_band_validation_1673.py index 5d4c57f7..a9fe8662 100644 --- a/xrspatial/geotiff/tests/test_band_validation_1673.py +++ b/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 diff --git a/xrspatial/geotiff/tests/test_dask_no_op_astype_1624.py b/xrspatial/geotiff/tests/test_dask_no_op_astype_1624.py index 5c931b3d..34724186 100644 --- a/xrspatial/geotiff/tests/test_dask_no_op_astype_1624.py +++ b/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() diff --git a/xrspatial/geotiff/tests/test_namespace_no_leak_1708.py b/xrspatial/geotiff/tests/test_namespace_no_leak_1708.py new file mode 100644 index 00000000..b64ee6de --- /dev/null +++ b/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