From f199e60ba92b9047787aee067d8685f0e5b3acc3 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Fri, 15 May 2026 10:03:32 -0700 Subject: [PATCH 1/3] geotiff: rename write_vrt's vrt_path kwarg to path (#1946) write_vrt's first positional kwarg was vrt_path while to_geotiff and write_geotiff_gpu use path, so callers passing write_vrt(path=...) hit TypeError on the only writer whose docstring advertises parity with the other two. Adds path as the canonical kwarg, keeps vrt_path as a deprecated alias (DeprecationWarning), and raises TypeError if both are supplied. Mirrors the existing crs / crs_wkt shim in the same writer (#1715). Tests pin positional and keyword forms, the deprecation warning on vrt_path, the both-supplied refusal, the cross-writer signature parity, and a round-trip equivalence between the two kwarg names. --- xrspatial/geotiff/__init__.py | 5 +- xrspatial/geotiff/_runtime.py | 8 + xrspatial/geotiff/_writers/vrt.py | 68 +++++- .../tests/test_write_vrt_path_kwarg_1946.py | 193 ++++++++++++++++++ 4 files changed, 264 insertions(+), 10 deletions(-) create mode 100644 xrspatial/geotiff/tests/test_write_vrt_path_kwarg_1946.py diff --git a/xrspatial/geotiff/__init__.py b/xrspatial/geotiff/__init__.py index 0af9307d..72649b94 100644 --- a/xrspatial/geotiff/__init__.py +++ b/xrspatial/geotiff/__init__.py @@ -26,8 +26,9 @@ write_geotiff_gpu(data, path, ...) GPU-only writer using nvCOMP. ``to_geotiff(..., gpu=True)`` calls this internally. -write_vrt(vrt_path, source_files, ...) - Generate a VRT mosaic XML from a list of GeoTIFF files. +write_vrt(path, source_files, ...) + Generate a VRT mosaic XML from a list of GeoTIFF files. ``vrt_path`` + is kept as a deprecated alias for ``path`` (#1946). """ from __future__ import annotations diff --git a/xrspatial/geotiff/_runtime.py b/xrspatial/geotiff/_runtime.py index aaed453c..fea0edff 100644 --- a/xrspatial/geotiff/_runtime.py +++ b/xrspatial/geotiff/_runtime.py @@ -39,6 +39,14 @@ # (forward verbatim to read_vrt). Mirrors the on_gpu_failure pattern. See # issue #1810. _MISSING_SOURCES_SENTINEL = object() +# ``write_vrt`` historically named its first positional kwarg ``vrt_path`` +# while ``to_geotiff`` / ``write_geotiff_gpu`` use ``path``. The deprecation +# shim adds ``path`` as the new name and accepts ``vrt_path`` with a +# DeprecationWarning. The sentinel pattern distinguishes "user passed +# vrt_path= explicitly" from "user passed nothing", which is the same +# rationale ``_CRS_WKT_DEPRECATED_SENTINEL`` documents above. See +# issue #1946. +_VRT_PATH_DEPRECATED_SENTINEL = object() # Spatial dim names recognised on 3D writer inputs. ``y``/``x`` are the diff --git a/xrspatial/geotiff/_writers/vrt.py b/xrspatial/geotiff/_writers/vrt.py index cbaf6480..9ef53e89 100644 --- a/xrspatial/geotiff/_writers/vrt.py +++ b/xrspatial/geotiff/_writers/vrt.py @@ -1,19 +1,24 @@ """VRT writer entry point. Wraps ``_vrt.write_vrt`` with the public ``write_vrt`` surface: -deprecation handling for ``crs_wkt`` (#1715), normalisation of the -``crs`` kwarg to WKT via ``_resolve_crs_to_wkt``, and the parity -docstring vs ``to_geotiff`` / ``write_geotiff_gpu``. +deprecation handling for ``crs_wkt`` (#1715) and ``vrt_path`` (#1946), +normalisation of the ``crs`` kwarg to WKT via ``_resolve_crs_to_wkt``, +and the parity docstring vs ``to_geotiff`` / ``write_geotiff_gpu``. """ from __future__ import annotations import warnings from .._crs import _resolve_crs_to_wkt -from .._runtime import _CRS_WKT_DEPRECATED_SENTINEL +from .._runtime import ( + _CRS_WKT_DEPRECATED_SENTINEL, + _VRT_PATH_DEPRECATED_SENTINEL, +) -def write_vrt(vrt_path: str, source_files: list[str], *, +def write_vrt(path: str | None = None, + source_files: list[str] | None = None, *, + vrt_path: str | None = _VRT_PATH_DEPRECATED_SENTINEL, relative: bool = True, crs: int | str | None = None, crs_wkt: str | None = _CRS_WKT_DEPRECATED_SENTINEL, @@ -22,10 +27,19 @@ def write_vrt(vrt_path: str, source_files: list[str], *, Parameters ---------- - vrt_path : str - Output .vrt file path. + path : str + Output .vrt file path. Mirrors the ``path`` kwarg on + ``to_geotiff`` and ``write_geotiff_gpu`` so the writer trio + shares a single destination-arg name (issue #1946). source_files : list of str Paths to the source GeoTIFF files. + vrt_path : str, optional + Deprecated alias for ``path``. Emits ``DeprecationWarning`` when + supplied; passing both ``path`` and ``vrt_path`` raises + ``TypeError``. Kept so existing callers (``write_vrt(vrt_path, + sources)`` positional or ``write_vrt(vrt_path=...)`` keyword) + keep working through the deprecation window. New code should + use ``path``. See issue #1946. relative : bool, optional Store source paths relative to the VRT file (default True). crs : int, str, or None, optional @@ -60,6 +74,44 @@ def write_vrt(vrt_path: str, source_files: list[str], *, # historic ``crs_wkt`` path; the new ``crs`` path normalises through # ``_resolve_crs_to_wkt`` before forwarding because the internal # writer still only speaks WKT. + # + # The ``path`` / ``vrt_path`` shim resolves the destination kwarg + # before any other processing so the rest of the function works + # uniformly against a single ``vrt_path`` local. ``path`` is the + # new name (parity with to_geotiff / write_geotiff_gpu); ``vrt_path`` + # is kept as a deprecated alias to preserve back-compat for callers + # using either positional ``write_vrt(vrt_path, sources)`` or + # keyword ``write_vrt(vrt_path=...)``. + vrt_path_passed = vrt_path is not _VRT_PATH_DEPRECATED_SENTINEL + if path is not None and vrt_path_passed: + # Both supplied is ambiguous regardless of whether the two values + # happen to be the same string. Refuse rather than silently + # picking one. Mirrors the same rule the ``crs`` / ``crs_wkt`` + # shim below applies. + raise TypeError( + "write_vrt: pass either 'path' or the deprecated 'vrt_path' " + "alias, not both.") + if vrt_path_passed: + warnings.warn( + "write_vrt(..., vrt_path=...) is deprecated; use path=... " + "instead. The kwarg was renamed for parity with to_geotiff " + "and write_geotiff_gpu, which already accept 'path' as the " + "destination kwarg.", + DeprecationWarning, + stacklevel=2, + ) + path = vrt_path + if path is None: + # Neither name supplied. Match the previous ``TypeError: missing + # required positional argument`` semantics by raising rather than + # forwarding a ``None`` into ``_write_vrt_internal`` (which would + # otherwise crash deep in ``os.path.dirname(os.path.abspath(None))``). + raise TypeError( + "write_vrt: missing required argument 'path'") + if source_files is None: + raise TypeError( + "write_vrt: missing required argument 'source_files'") + crs_wkt_passed = crs_wkt is not _CRS_WKT_DEPRECATED_SENTINEL if crs is not None and crs_wkt_passed: # Both supplied is ambiguous regardless of whether the WKT happens @@ -83,7 +135,7 @@ def write_vrt(vrt_path: str, source_files: list[str], *, from .._vrt import write_vrt as _write_vrt_internal return _write_vrt_internal( - vrt_path, source_files, + path, source_files, relative=relative, crs_wkt=resolved_wkt, nodata=nodata, diff --git a/xrspatial/geotiff/tests/test_write_vrt_path_kwarg_1946.py b/xrspatial/geotiff/tests/test_write_vrt_path_kwarg_1946.py new file mode 100644 index 00000000..baafe803 --- /dev/null +++ b/xrspatial/geotiff/tests/test_write_vrt_path_kwarg_1946.py @@ -0,0 +1,193 @@ +"""Regression test for #1946: write_vrt accepts ``path`` for parity +with ``to_geotiff`` / ``write_geotiff_gpu``. + +The api-consistency sweep on 2026-05-15 flagged that ``write_vrt`` was +the only writer in ``xrspatial.geotiff`` whose destination kwarg was +named ``vrt_path`` while the sibling writers use ``path``. The fix adds +``path`` as the canonical kwarg and keeps ``vrt_path`` as a deprecated +alias. + +This module pins: + +* Positional ``write_vrt(path, sources)`` works (back-compat with the + previous ``write_vrt(vrt_path, sources)`` positional form). +* Keyword ``write_vrt(path=..., source_files=...)`` works (the new + canonical form). +* Keyword ``write_vrt(vrt_path=...)`` still works and emits + ``DeprecationWarning``. +* Passing both ``path`` and ``vrt_path`` raises ``TypeError``. +* The signature exposes ``path`` as the first positional, matching + ``to_geotiff`` / ``write_geotiff_gpu``. +* The deprecation shim does NOT warn when ``path`` is used. +* Omitting both names raises ``TypeError`` (preserves the pre-#1946 + required-argument semantics). +""" +from __future__ import annotations + +import inspect +import os +import warnings + +import numpy as np +import pytest +import xarray as xr + +from xrspatial.geotiff import ( + read_vrt, + to_geotiff, + write_geotiff_gpu, + write_vrt, +) + + +def _build_source_tif(tmp_path, name='src.tif'): + """Create a small GeoTIFF used as the VRT's source file.""" + arr = np.arange(8 * 8, dtype=np.float32).reshape(8, 8) + da = xr.DataArray( + arr, dims=['y', 'x'], + coords={'y': np.arange(8.0, 0, -1), 'x': np.arange(8.0)}, + attrs={'crs': 4326, 'transform': (1.0, 0, 0.0, 0, -1.0, 8.0)}, + ) + p = str(tmp_path / name) + to_geotiff(da, p) + return p + + +def test_write_vrt_signature_first_arg_is_path(): + """Signature parity with to_geotiff / write_geotiff_gpu. + + The api-consistency sweep cares specifically about + ``inspect.signature``: IDE autocomplete, mypy, and Sphinx-rendered + docs all read the same source. Pinning the first param name here + catches any future re-rename that re-introduces the drift. + """ + sig = inspect.signature(write_vrt) + params = list(sig.parameters) + # ``path`` is the new canonical name, ``source_files`` follows. + # ``vrt_path`` is kept as a keyword-only deprecated alias. + assert params[0] == 'path' + assert params[1] == 'source_files' + assert 'vrt_path' in params + # ``vrt_path`` is keyword-only (the alias should never be used + # positionally going forward). + assert sig.parameters['vrt_path'].kind == inspect.Parameter.KEYWORD_ONLY + + +def test_write_vrt_positional_path_works(tmp_path): + """Positional ``write_vrt(path, sources)`` is unchanged. + + Existing callers ``write_vrt(some_path, sources)`` keep working + after the rename because the new ``path`` parameter sits where + ``vrt_path`` used to be. No deprecation warning should fire. + """ + src = _build_source_tif(tmp_path) + out = str(tmp_path / 'out.vrt') + with warnings.catch_warnings(): + warnings.simplefilter('error', DeprecationWarning) + result = write_vrt(out, [src]) + assert result == out + assert os.path.exists(out) + + +def test_write_vrt_path_kwarg_works(tmp_path): + """Keyword ``write_vrt(path=..., source_files=...)`` works. + + A caller who passes everything by keyword (no positional args) + cannot reach the function before #1946 because ``path`` did not + exist; this is the path-symmetric counterpart to the existing + ``write_vrt(vrt_path=...)`` test below. + """ + src = _build_source_tif(tmp_path) + out = str(tmp_path / 'out.vrt') + with warnings.catch_warnings(): + warnings.simplefilter('error', DeprecationWarning) + result = write_vrt(path=out, source_files=[src]) + assert result == out + assert os.path.exists(out) + + +def test_write_vrt_vrt_path_kwarg_emits_deprecation_warning(tmp_path): + """``vrt_path=...`` works but emits ``DeprecationWarning``. + + Mirrors the existing ``crs_wkt`` deprecation in the same writer + (#1715): old name still works, but caller sees a clear migration + hint via the warning. + """ + src = _build_source_tif(tmp_path) + out = str(tmp_path / 'out.vrt') + with pytest.warns(DeprecationWarning, match='vrt_path'): + result = write_vrt(vrt_path=out, source_files=[src]) + assert result == out + assert os.path.exists(out) + + +def test_write_vrt_path_and_vrt_path_together_raises(tmp_path): + """Both names supplied is ambiguous; refuse to pick one. + + Mirrors the ``crs`` / ``crs_wkt`` rule documented in the existing + write_vrt source: passing both is rejected with TypeError + regardless of whether the two values happen to match. + """ + src = _build_source_tif(tmp_path) + out = str(tmp_path / 'out.vrt') + with pytest.raises(TypeError, match="path.*vrt_path"): + write_vrt(path=out, vrt_path=out, source_files=[src]) + + +def test_write_vrt_no_path_raises(tmp_path): + """Neither ``path`` nor ``vrt_path`` -> TypeError. + + Before the shim, omitting the first positional argument raised + ``TypeError: missing 1 required positional argument`` from CPython. + The shim adds a default of ``None`` so the kwarg-only positional + no longer triggers that automatic check; the explicit raise inside + the shim preserves the pre-#1946 error semantics. + """ + src = _build_source_tif(tmp_path) + with pytest.raises(TypeError, match='path'): + write_vrt(source_files=[src]) + + +def test_write_vrt_first_arg_name_matches_writer_trio(): + """Cross-sibling consistency: all three writers use the same + destination kwarg name. + + The deep-sweep-api-consistency sweep keeps adding to the writer + trio's parity contract. Pin the rule here so future re-renames + that split the trio again will trip a test. + """ + eager_first = list( + inspect.signature(to_geotiff).parameters + )[1] # data, path -> index 1 + gpu_first = list( + inspect.signature(write_geotiff_gpu).parameters + )[1] + vrt_first = list( + inspect.signature(write_vrt).parameters + )[0] # path, source_files -> index 0 + assert eager_first == 'path' + assert gpu_first == 'path' + assert vrt_first == 'path' + + +def test_write_vrt_path_round_trip_matches_old(tmp_path): + """The written VRT decodes the same regardless of which kwarg name + the caller used. + + Smoke test that the shim does not silently drop or re-route any of + the other kwargs while resolving ``path`` vs ``vrt_path``. + """ + src = _build_source_tif(tmp_path) + out_new = str(tmp_path / 'out_new.vrt') + out_old = str(tmp_path / 'out_old.vrt') + + write_vrt(out_new, [src]) + with warnings.catch_warnings(): + # ignore the deprecation; we still need the legacy path to + # produce a byte-identical mosaic. + warnings.simplefilter('ignore', DeprecationWarning) + write_vrt(vrt_path=out_old, source_files=[src]) + + a_new = read_vrt(out_new) + a_old = read_vrt(out_old) + np.testing.assert_array_equal(np.asarray(a_new), np.asarray(a_old)) From 38a4e767a314e34377c5bb6e9b9817b217d3d5f0 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Fri, 15 May 2026 10:54:29 -0700 Subject: [PATCH 2/3] geotiff: address PR #1962 review / CI fixes --- .../tests/test_signature_annotations_1654.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/xrspatial/geotiff/tests/test_signature_annotations_1654.py b/xrspatial/geotiff/tests/test_signature_annotations_1654.py index 585fb77f..4430ae4d 100644 --- a/xrspatial/geotiff/tests/test_signature_annotations_1654.py +++ b/xrspatial/geotiff/tests/test_signature_annotations_1654.py @@ -83,10 +83,22 @@ def test_write_geotiff_gpu_path_annotated(): assert 'BinaryIO' in ann +def test_write_vrt_path_annotated(): + """``write_vrt(path, ...)`` is str-only (VRT writes are path-only by + design; no file-like buffer support). After #1946 the canonical name + is ``path`` (parity with ``to_geotiff`` / ``write_geotiff_gpu``); the + annotation is ``str | None`` because the parameter carries a default + of ``None`` so the deprecation shim can detect omission and route + callers using the legacy ``vrt_path=`` alias.""" + assert _annotation(write_vrt, 'path') == 'str | None' + + def test_write_vrt_vrt_path_annotated(): - """``write_vrt(vrt_path, ...)`` stays str-only (VRT writes are - path-only by design; no file-like buffer support).""" - assert _annotation(write_vrt, 'vrt_path') == 'str' + """The deprecated ``vrt_path`` alias keeps the same ``str | None`` + annotation as ``path`` (str-only at the type level; ``None`` only + appears because the sentinel default lets the shim detect omission). + Pinned so a future re-rename does not silently widen the alias.""" + assert _annotation(write_vrt, 'vrt_path') == 'str | None' # --- source: str or BinaryIO (open_geotiff is the public dispatch) --- From 7b83a552b9bf91cde9416feeb81ad195501950e9 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Fri, 15 May 2026 12:43:34 -0700 Subject: [PATCH 3/3] geotiff: address PR #1962 review (sentinel for write_vrt path, docstring TypeError note) Switch write_vrt's path kwarg from a None default to a private _VRT_PATH_MISSING_SENTINEL so an explicit write_vrt(path=None, ...) or positional write_vrt(None, sources) is rejected with a clear TypeError naming the offending kwarg, instead of falling through to the "missing required argument" branch. Mirrors the existing _CRS_WKT_DEPRECATED_SENTINEL pattern in the same writer. Also mirror the crs_wkt entry's "passing both raises TypeError" wording in the write_vrt entry of the geotiff Public API docstring. Updates the test_write_vrt_path_annotated annotation assertion from 'str | None' to 'str' to match the new sentinel-typed default and adds two new regression tests pinning the keyword and positional explicit-None cases. --- xrspatial/geotiff/__init__.py | 3 +- xrspatial/geotiff/_runtime.py | 7 ++++ xrspatial/geotiff/_writers/vrt.py | 20 +++++++++--- .../tests/test_signature_annotations_1654.py | 12 ++++--- .../tests/test_write_vrt_path_kwarg_1946.py | 32 +++++++++++++++++-- 5 files changed, 61 insertions(+), 13 deletions(-) diff --git a/xrspatial/geotiff/__init__.py b/xrspatial/geotiff/__init__.py index 72649b94..467ee96a 100644 --- a/xrspatial/geotiff/__init__.py +++ b/xrspatial/geotiff/__init__.py @@ -28,7 +28,8 @@ internally. write_vrt(path, source_files, ...) Generate a VRT mosaic XML from a list of GeoTIFF files. ``vrt_path`` - is kept as a deprecated alias for ``path`` (#1946). + is kept as a deprecated alias for ``path``; passing both ``path`` and + ``vrt_path`` raises ``TypeError`` (#1946). """ from __future__ import annotations diff --git a/xrspatial/geotiff/_runtime.py b/xrspatial/geotiff/_runtime.py index fea0edff..402558c8 100644 --- a/xrspatial/geotiff/_runtime.py +++ b/xrspatial/geotiff/_runtime.py @@ -47,6 +47,13 @@ # rationale ``_CRS_WKT_DEPRECATED_SENTINEL`` documents above. See # issue #1946. _VRT_PATH_DEPRECATED_SENTINEL = object() +# ``write_vrt`` also needs to distinguish "user passed path= explicitly" +# (including an explicit ``path=None``, which is an error) from "user +# passed nothing" (fall through to the ``vrt_path`` shim). Without this +# sentinel, ``write_vrt(None, sources)`` silently fell through to the +# ``path is None`` branch and raised a "missing required argument" +# TypeError for the wrong reason. See PR #1962 review. +_VRT_PATH_MISSING_SENTINEL = object() # Spatial dim names recognised on 3D writer inputs. ``y``/``x`` are the diff --git a/xrspatial/geotiff/_writers/vrt.py b/xrspatial/geotiff/_writers/vrt.py index 9ef53e89..a2411cf3 100644 --- a/xrspatial/geotiff/_writers/vrt.py +++ b/xrspatial/geotiff/_writers/vrt.py @@ -13,10 +13,11 @@ from .._runtime import ( _CRS_WKT_DEPRECATED_SENTINEL, _VRT_PATH_DEPRECATED_SENTINEL, + _VRT_PATH_MISSING_SENTINEL, ) -def write_vrt(path: str | None = None, +def write_vrt(path: str = _VRT_PATH_MISSING_SENTINEL, source_files: list[str] | None = None, *, vrt_path: str | None = _VRT_PATH_DEPRECATED_SENTINEL, relative: bool = True, @@ -82,8 +83,9 @@ def write_vrt(path: str | None = None, # is kept as a deprecated alias to preserve back-compat for callers # using either positional ``write_vrt(vrt_path, sources)`` or # keyword ``write_vrt(vrt_path=...)``. + path_passed = path is not _VRT_PATH_MISSING_SENTINEL vrt_path_passed = vrt_path is not _VRT_PATH_DEPRECATED_SENTINEL - if path is not None and vrt_path_passed: + if path_passed and vrt_path_passed: # Both supplied is ambiguous regardless of whether the two values # happen to be the same string. Refuse rather than silently # picking one. Mirrors the same rule the ``crs`` / ``crs_wkt`` @@ -101,13 +103,21 @@ def write_vrt(path: str | None = None, stacklevel=2, ) path = vrt_path - if path is None: + elif not path_passed: # Neither name supplied. Match the previous ``TypeError: missing # required positional argument`` semantics by raising rather than - # forwarding a ``None`` into ``_write_vrt_internal`` (which would - # otherwise crash deep in ``os.path.dirname(os.path.abspath(None))``). + # forwarding the sentinel into ``_write_vrt_internal``. raise TypeError( "write_vrt: missing required argument 'path'") + if path is None: + # Explicit ``path=None`` (including positional ``write_vrt(None, + # sources)``) is rejected up front so the error message names the + # offending kwarg instead of crashing deep in + # ``os.path.dirname(os.path.abspath(None))``. The sentinel default + # on ``path`` is what lets us distinguish this case from "caller + # passed nothing" above. + raise TypeError( + "write_vrt: 'path' must be a str, got None") if source_files is None: raise TypeError( "write_vrt: missing required argument 'source_files'") diff --git a/xrspatial/geotiff/tests/test_signature_annotations_1654.py b/xrspatial/geotiff/tests/test_signature_annotations_1654.py index 4430ae4d..246a14f7 100644 --- a/xrspatial/geotiff/tests/test_signature_annotations_1654.py +++ b/xrspatial/geotiff/tests/test_signature_annotations_1654.py @@ -86,11 +86,13 @@ def test_write_geotiff_gpu_path_annotated(): def test_write_vrt_path_annotated(): """``write_vrt(path, ...)`` is str-only (VRT writes are path-only by design; no file-like buffer support). After #1946 the canonical name - is ``path`` (parity with ``to_geotiff`` / ``write_geotiff_gpu``); the - annotation is ``str | None`` because the parameter carries a default - of ``None`` so the deprecation shim can detect omission and route - callers using the legacy ``vrt_path=`` alias.""" - assert _annotation(write_vrt, 'path') == 'str | None' + is ``path`` (parity with ``to_geotiff`` / ``write_geotiff_gpu``). + The annotation is plain ``str``: the default value is a private + sentinel (not ``None``) so the deprecation shim can distinguish + ``write_vrt(path=None, ...)`` (rejected with TypeError) from a + caller who omitted ``path`` entirely (routed through the ``vrt_path`` + alias). See PR #1962 review.""" + assert _annotation(write_vrt, 'path') == 'str' def test_write_vrt_vrt_path_annotated(): diff --git a/xrspatial/geotiff/tests/test_write_vrt_path_kwarg_1946.py b/xrspatial/geotiff/tests/test_write_vrt_path_kwarg_1946.py index baafe803..b1add2ce 100644 --- a/xrspatial/geotiff/tests/test_write_vrt_path_kwarg_1946.py +++ b/xrspatial/geotiff/tests/test_write_vrt_path_kwarg_1946.py @@ -139,8 +139,8 @@ def test_write_vrt_no_path_raises(tmp_path): Before the shim, omitting the first positional argument raised ``TypeError: missing 1 required positional argument`` from CPython. - The shim adds a default of ``None`` so the kwarg-only positional - no longer triggers that automatic check; the explicit raise inside + The shim adds a sentinel default so the kwarg-only positional no + longer triggers that automatic check; the explicit raise inside the shim preserves the pre-#1946 error semantics. """ src = _build_source_tif(tmp_path) @@ -148,6 +148,34 @@ def test_write_vrt_no_path_raises(tmp_path): write_vrt(source_files=[src]) +def test_write_vrt_explicit_path_none_raises(tmp_path): + """``write_vrt(path=None, ...)`` is rejected with TypeError. + + The sentinel-default pattern (#1962 review) distinguishes "caller + passed nothing" (sentinel) from "caller passed None explicitly". + Explicit ``None`` is a bug in the caller's code, not a request to + fall through to the deprecated ``vrt_path`` alias, so the shim + raises with a clear message that names the offending kwarg. + """ + src = _build_source_tif(tmp_path) + with pytest.raises(TypeError, match="'path'.*None"): + write_vrt(path=None, source_files=[src]) + + +def test_write_vrt_positional_none_raises(tmp_path): + """Positional ``write_vrt(None, sources)`` is rejected with TypeError. + + Same rationale as the keyword case: an explicit positional ``None`` + is rejected up front instead of crashing deep in + ``os.path.dirname(os.path.abspath(None))``. Pinned because the + pre-#1962 code accepted positional ``None`` and raised the wrong + "missing required argument" error. + """ + src = _build_source_tif(tmp_path) + with pytest.raises(TypeError, match="'path'.*None"): + write_vrt(None, [src]) + + def test_write_vrt_first_arg_name_matches_writer_trio(): """Cross-sibling consistency: all three writers use the same destination kwarg name.