Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions xrspatial/geotiff/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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``
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
16 changes: 9 additions & 7 deletions xrspatial/geotiff/tests/test_band_validation_1673.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"):
Expand All @@ -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.
Expand All @@ -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"):
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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

Expand All @@ -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

Expand Down
7 changes: 6 additions & 1 deletion xrspatial/geotiff/tests/test_dask_no_op_astype_1624.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
64 changes: 64 additions & 0 deletions xrspatial/geotiff/tests/test_namespace_no_leak_1708.py
Original file line number Diff line number Diff line change
@@ -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
Loading