diff --git a/xrspatial/geotiff/__init__.py b/xrspatial/geotiff/__init__.py index 0908ccfc8..3a64c1ebc 100644 --- a/xrspatial/geotiff/__init__.py +++ b/xrspatial/geotiff/__init__.py @@ -61,6 +61,17 @@ # 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 ._runtime import ( + GeoTIFFFallbackWarning, + _CRS_WKT_DEPRECATED_SENTINEL, + _GPU_DEPRECATED_SENTINEL, + _MISSING_SOURCES_SENTINEL, + _ON_GPU_FAILURE_SENTINEL, + _X_DIM_NAMES, + _Y_DIM_NAMES, + _geotiff_strict_mode, + _gpu_fallback_warning_message, +) from ._writer import write # All names below are part of the supported public API. ``plot_geotiff`` @@ -79,38 +90,6 @@ ] -# Sentinels distinguishing "user passed this kwarg explicitly" from "user -# passed nothing". A plain default of None does not work because None is -# itself a value a caller could supply. ``read_geotiff_gpu`` needs both -# sentinels so it can tell whether the deprecated ``gpu=`` and the new -# ``on_gpu_failure=`` were *each* supplied, and refuse the ambiguous -# both-supplied case regardless of which values were chosen. -# ``open_geotiff`` also uses ``_ON_GPU_FAILURE_SENTINEL`` to distinguish -# "caller never set on_gpu_failure" (default sentinel: skip forwarding so -# the read_geotiff_gpu signature default applies) from "caller set -# on_gpu_failure=" (forward verbatim). -_GPU_DEPRECATED_SENTINEL = object() -_ON_GPU_FAILURE_SENTINEL = object() -# ``write_vrt`` needs to distinguish "user passed crs_wkt= explicitly" -# (deprecation path) from "user passed nothing" (no warning, pick CRS -# from the first source). A plain default of None does not work because -# None is itself a value a caller could supply alongside crs=. See -# issue #1715. -_CRS_WKT_DEPRECATED_SENTINEL = object() -# ``open_geotiff`` needs to tell "caller never set missing_sources" (default -# sentinel: skip forwarding so the read_vrt default applies, and reject the -# kwarg up front for non-VRT sources) from "caller set missing_sources=" -# (forward verbatim to read_vrt). Mirrors the on_gpu_failure pattern. See -# issue #1810. -_MISSING_SOURCES_SENTINEL = object() - -# Spatial dim names recognised on 3D writer inputs. ``y``/``x`` are the -# canonical TIFF axes; aliases are accepted so a user who happens to use -# ``lat``/``lon`` or ``row``/``col`` is not bounced by the validator below. -_Y_DIM_NAMES = ('y', 'lat', 'latitude', 'row') -_X_DIM_NAMES = ('x', 'lon', 'longitude', 'col') - - def _validate_3d_writer_dims(dims) -> None: """Reject ambiguous 3D writer inputs (issue #1812). @@ -157,51 +136,6 @@ def _validate_3d_writer_dims(dims) -> None: ) -class GeoTIFFFallbackWarning(UserWarning): - """Warning emitted when a geotiff helper falls back to a slower path. - - Raised in the same call sites that would silently return ``None`` under - the historic ``except Exception: return None`` pattern. See issue #1662 - for the audit and the ``XRSPATIAL_GEOTIFF_STRICT=1`` env var that - promotes these warnings to exceptions. - """ - - -def _geotiff_strict_mode() -> bool: - """Return True when ``XRSPATIAL_GEOTIFF_STRICT`` is set to a truthy value. - - Strict mode promotes the silent fallbacks audited in issue #1662 into - raised exceptions. Useful in CI to catch GPU-path or VRT regressions - that would otherwise hide behind a CPU fallback or a missing tile. - """ - return os.environ.get( - 'XRSPATIAL_GEOTIFF_STRICT', '').lower() in ('1', 'true', 'yes') - - -def _gpu_fallback_warning_message(auto_detected: bool, exc: BaseException) -> str: - """Build the ``to_geotiff`` GPU-to-CPU fallback warning text. - - ``to_geotiff`` reaches the GPU writer two ways: an explicit - ``gpu=True`` argument, or the auto-detect branch when ``gpu is - None`` and the data lives on a CuPy device. The wording differs - because blaming the fallback on a flag the caller never set sends - them to fix the wrong thing. Both routes share the exception - payload format so callers can grep ``type(e).__name__: e`` either - way. - """ - suffix = f"({type(exc).__name__}: {exc})." - if auto_detected: - return ( - "Data is on the GPU and was routed to the GPU writer, but " - "the writer is unavailable; falling back to CPU and copying " - "the array to host. " + suffix - ) - return ( - "to_geotiff(gpu=True) was requested but the GPU writer is " - "unavailable; falling back to CPU. " + suffix - ) - - def _wkt_to_epsg(wkt_or_proj: str) -> int | None: """Try to extract an EPSG code from a WKT or PROJ string. diff --git a/xrspatial/geotiff/_runtime.py b/xrspatial/geotiff/_runtime.py new file mode 100644 index 000000000..aaed453c8 --- /dev/null +++ b/xrspatial/geotiff/_runtime.py @@ -0,0 +1,93 @@ +"""Geotiff module-level runtime state: sentinels, fallback warning, strict mode. + +These live in their own module so that backend extractions can import +a single canonical binding for each sentinel and helper without +threading them through ``__init__.py``. Sentinel object identity is +preserved by Python's module cache: every import of this module +returns the same module instance, so ``_GPU_DEPRECATED_SENTINEL is +other._GPU_DEPRECATED_SENTINEL`` resolves correctly regardless of +which caller imported it. + +See issue #1813 step 2 for the rationale; PR for issue #1880. +""" +from __future__ import annotations + +import os + + +# Sentinels distinguishing "user passed this kwarg explicitly" from "user +# passed nothing". A plain default of None does not work because None is +# itself a value a caller could supply. ``read_geotiff_gpu`` needs both +# sentinels so it can tell whether the deprecated ``gpu=`` and the new +# ``on_gpu_failure=`` were *each* supplied, and refuse the ambiguous +# both-supplied case regardless of which values were chosen. +# ``open_geotiff`` also uses ``_ON_GPU_FAILURE_SENTINEL`` to distinguish +# "caller never set on_gpu_failure" (default sentinel: skip forwarding so +# the read_geotiff_gpu signature default applies) from "caller set +# on_gpu_failure=" (forward verbatim). +_GPU_DEPRECATED_SENTINEL = object() +_ON_GPU_FAILURE_SENTINEL = object() +# ``write_vrt`` needs to distinguish "user passed crs_wkt= explicitly" +# (deprecation path) from "user passed nothing" (no warning, pick CRS +# from the first source). A plain default of None does not work because +# None is itself a value a caller could supply alongside crs=. See +# issue #1715. +_CRS_WKT_DEPRECATED_SENTINEL = object() +# ``open_geotiff`` needs to tell "caller never set missing_sources" (default +# sentinel: skip forwarding so the read_vrt default applies, and reject the +# kwarg up front for non-VRT sources) from "caller set missing_sources=" +# (forward verbatim to read_vrt). Mirrors the on_gpu_failure pattern. See +# issue #1810. +_MISSING_SOURCES_SENTINEL = object() + + +# Spatial dim names recognised on 3D writer inputs. ``y``/``x`` are the +# canonical TIFF axes; aliases are accepted so a user who happens to use +# ``lat``/``lon`` or ``row``/``col`` is not bounced by the validator. +_Y_DIM_NAMES = ('y', 'lat', 'latitude', 'row') +_X_DIM_NAMES = ('x', 'lon', 'longitude', 'col') + + +class GeoTIFFFallbackWarning(UserWarning): + """Warning emitted when a geotiff helper falls back to a slower path. + + Raised in the same call sites that would silently return ``None`` under + the historic ``except Exception: return None`` pattern. See issue #1662 + for the audit and the ``XRSPATIAL_GEOTIFF_STRICT=1`` env var that + promotes these warnings to exceptions. + """ + + +def _geotiff_strict_mode() -> bool: + """Return True when ``XRSPATIAL_GEOTIFF_STRICT`` is set to a truthy value. + + Strict mode promotes the silent fallbacks audited in issue #1662 into + raised exceptions. Useful in CI to catch GPU-path or VRT regressions + that would otherwise hide behind a CPU fallback or a missing tile. + """ + return os.environ.get( + 'XRSPATIAL_GEOTIFF_STRICT', '').lower() in ('1', 'true', 'yes') + + +def _gpu_fallback_warning_message(auto_detected: bool, exc: BaseException) -> str: + """Build the ``to_geotiff`` GPU-to-CPU fallback warning text. + + ``to_geotiff`` reaches the GPU writer two ways: an explicit + ``gpu=True`` argument, or the auto-detect branch when ``gpu is + None`` and the data lives on a CuPy device. The wording differs + because blaming the fallback on a flag the caller never set sends + them to fix the wrong thing. Both routes share the exception + payload format so callers can grep ``type(e).__name__: e`` either + way. + """ + suffix = f"({type(exc).__name__}: {exc})." + if auto_detected: + return ( + "Data is on the GPU and was routed to the GPU writer, but " + "the writer is unavailable; falling back to CPU and copying " + "the array to host. " + suffix + ) + return ( + "to_geotiff(gpu=True) was requested but the GPU writer is " + "unavailable; falling back to CPU. " + suffix + ) diff --git a/xrspatial/geotiff/tests/test_runtime_sentinels_identity_1880.py b/xrspatial/geotiff/tests/test_runtime_sentinels_identity_1880.py new file mode 100644 index 000000000..b1af6796e --- /dev/null +++ b/xrspatial/geotiff/tests/test_runtime_sentinels_identity_1880.py @@ -0,0 +1,94 @@ +"""Identity contract: sentinels survive the move from __init__.py to _runtime.py. + +PR for issue #1880 (step 2 of #1813) extracted four module-level +sentinels, a UserWarning subclass, the strict-mode helper, and the +GPU-fallback message helper from ``xrspatial/geotiff/__init__.py`` +into ``xrspatial/geotiff/_runtime.py``. ``__init__.py`` keeps every +name importable via re-export. + +If a future refactor accidentally rebinds one of these names to a +fresh ``object()`` (or shadows ``GeoTIFFFallbackWarning`` with a local +class), every ``is`` comparison against the sentinel inside the entry +points would silently start failing -- ``open_geotiff`` would no +longer recognise the canonical sentinel and would treat "caller passed +no kwarg" as "caller passed the default value". This file pins the +contract: the names imported through ``xrspatial.geotiff`` and through +``xrspatial.geotiff._runtime`` are the same Python object. +""" +from __future__ import annotations + +import xrspatial.geotiff as geotiff_pkg +from xrspatial.geotiff import _runtime + + +def test_gpu_deprecated_sentinel_is_singleton(): + assert geotiff_pkg._GPU_DEPRECATED_SENTINEL is _runtime._GPU_DEPRECATED_SENTINEL + + +def test_on_gpu_failure_sentinel_is_singleton(): + assert geotiff_pkg._ON_GPU_FAILURE_SENTINEL is _runtime._ON_GPU_FAILURE_SENTINEL + + +def test_crs_wkt_deprecated_sentinel_is_singleton(): + assert geotiff_pkg._CRS_WKT_DEPRECATED_SENTINEL is \ + _runtime._CRS_WKT_DEPRECATED_SENTINEL + + +def test_missing_sources_sentinel_is_singleton(): + assert geotiff_pkg._MISSING_SOURCES_SENTINEL is \ + _runtime._MISSING_SOURCES_SENTINEL + + +def test_fallback_warning_class_is_singleton(): + """``GeoTIFFFallbackWarning`` is the same class through both import paths. + + This is the only re-exported name from ``_runtime`` that is in + ``__all__``. A duplicate class would still print the right name in + a ``warns(GeoTIFFFallbackWarning)`` context but ``issubclass`` + chains in user code would break. + """ + assert geotiff_pkg.GeoTIFFFallbackWarning is _runtime.GeoTIFFFallbackWarning + + +def test_dim_name_tuples_are_singleton(): + assert geotiff_pkg._Y_DIM_NAMES is _runtime._Y_DIM_NAMES + assert geotiff_pkg._X_DIM_NAMES is _runtime._X_DIM_NAMES + + +def test_strict_mode_helper_is_singleton(): + assert geotiff_pkg._geotiff_strict_mode is _runtime._geotiff_strict_mode + + +def test_gpu_fallback_warning_message_is_singleton(): + assert geotiff_pkg._gpu_fallback_warning_message is \ + _runtime._gpu_fallback_warning_message + + +def test_strict_mode_env_var_round_trips(monkeypatch): + """The strict-mode helper still reads the env var after the move. + + Guards against an accidental hard-coded return value or wrong env + var name introduced during the relocation. + """ + monkeypatch.setenv("XRSPATIAL_GEOTIFF_STRICT", "1") + assert geotiff_pkg._geotiff_strict_mode() is True + monkeypatch.setenv("XRSPATIAL_GEOTIFF_STRICT", "true") + assert geotiff_pkg._geotiff_strict_mode() is True + monkeypatch.setenv("XRSPATIAL_GEOTIFF_STRICT", "0") + assert geotiff_pkg._geotiff_strict_mode() is False + monkeypatch.delenv("XRSPATIAL_GEOTIFF_STRICT", raising=False) + assert geotiff_pkg._geotiff_strict_mode() is False + + +def test_fallback_message_includes_exception_type_and_message(): + """The two GPU-fallback wording branches both surface the exception.""" + exc = RuntimeError("nvcomp not installed") + explicit = geotiff_pkg._gpu_fallback_warning_message( + auto_detected=False, exc=exc) + auto = geotiff_pkg._gpu_fallback_warning_message( + auto_detected=True, exc=exc) + for msg in (explicit, auto): + assert "RuntimeError" in msg + assert "nvcomp not installed" in msg + assert "to_geotiff(gpu=True) was requested" in explicit + assert "Data is on the GPU" in auto