Skip to content

Commit b337d38

Browse files
committed
write_vrt: accept crs= for parity with to_geotiff/write_geotiff_gpu (#1715)
The api-consistency sweep on 2026-05-12 found ``write_vrt`` was the only writer in ``xrspatial.geotiff`` using ``crs_wkt`` instead of ``crs``: to_geotiff(data, path, *, crs: int | str | None = None, ...) write_geotiff_gpu(data, path, *, crs: int | str | None = None, ...) write_vrt(vrt_path, source_files, *, crs_wkt: str | None = None, ...) This broke the "forward the same kwargs to whichever writer matches the output extension" pattern. Generic write-wrapper code had to special-case VRT, convert EPSG int -> WKT for that path, and remember that one writer in the trio used a different name. Reader-side ``attrs['crs']`` is the canonical EPSG-int surface, so the asymmetry is also visible in round-trip code. This change adds ``crs`` as the canonical kwarg with the same ``int | str | None`` shape as the sibling writers. ``crs_wkt`` is kept as a deprecated alias for backward compatibility: * ``crs=`` accepts int (EPSG), str (WKT or PROJ), or None. * ``crs_wkt=`` still works but emits ``DeprecationWarning``. * Passing both raises ``TypeError`` rather than silently picking one. * Omitting both (the most common call shape) emits NO warning. The wrapper normalises any non-None input through the new ``_resolve_crs_to_wkt`` helper before forwarding to the internal ``_vrt.write_vrt`` (which only speaks WKT). EPSG int and PROJ-string inputs are routed through pyproj's ``CRS.from_user_input(...).to_wkt()``; already-WKT strings (recognised by the PROJCS/GEOGCS/PROJCRS/GEOGCRS/ COMPD_CS/COMPOUNDCRS prefix) pass through verbatim so the existing WKT-passthrough behaviour for the XML-escape tests is preserved. The deprecation shim follows the same sentinel-default pattern the deprecated ``gpu=`` alias uses on ``read_geotiff_gpu``: a private ``_CRS_WKT_DEPRECATED_SENTINEL`` distinguishes "user passed nothing" (silent) from "user passed ``crs_wkt=None`` explicitly" (warn). test_write_vrt_crs_1715.py adds 12 tests pinning the new contract: * signature pins (``crs`` present, default None, annotation ``int | str | None``); * runtime: ``crs=<EPSG int>``, ``crs=<WKT string>``, ``crs=None``, no-crs-kwarg-no-warning, all round-trip back to ``attrs['crs']``; * deprecation shim: ``crs_wkt=<wkt>`` warns + still works, ``crs_wkt=None`` (explicit) warns, both-kwargs raises ``TypeError``; * cross-writer parity: ``crs`` annotation matches on all three writers; * negative tests: invalid type / unparseable string raise on the public wrapper. Existing tests that were calling ``write_vrt(..., crs_wkt=...)`` through the public wrapper are migrated to ``crs=`` so the suite no longer emits spurious DeprecationWarnings: * ``test_polish_1488.py``: ``crs=None`` replaces ``crs_wkt=None``. * ``test_kwarg_behaviour_2026_05_12.py::TestWriteVrtCrsWktBehaviour``: three sub-tests migrated from ``crs_wkt=`` to ``crs=``. * ``test_signature_parity_1631.py``: signature test updated to assert both kwargs and uses ``crs=None`` in the runtime call. ``test_vrt_xml_escape_1607.py`` keeps using ``crs_wkt=`` because it imports from the internal ``xrspatial.geotiff._vrt`` (not the public wrapper) and exercises raw XML-escape semantics; the rename does not apply there. Closes #1715.
1 parent 1624d13 commit b337d38

5 files changed

Lines changed: 402 additions & 21 deletions

File tree

xrspatial/geotiff/__init__.py

Lines changed: 130 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@
7474
# on_gpu_failure=<value>" (forward verbatim).
7575
_GPU_DEPRECATED_SENTINEL = object()
7676
_ON_GPU_FAILURE_SENTINEL = object()
77+
# ``write_vrt`` needs to distinguish "user passed crs_wkt= explicitly"
78+
# (deprecation path) from "user passed nothing" (no warning, pick CRS
79+
# from the first source). A plain default of None does not work because
80+
# None is itself a value a caller could supply alongside crs=. See
81+
# issue #1715.
82+
_CRS_WKT_DEPRECATED_SENTINEL = object()
7783

7884
# Names of dims that ``to_geotiff`` / ``write_geotiff_gpu`` treat as the
7985
# non-spatial band axis. Used both to remap ``(band, y, x)`` inputs to
@@ -153,6 +159,91 @@ def _wkt_to_epsg(wkt_or_proj: str) -> int | None:
153159
return None
154160

155161

162+
def _resolve_crs_to_wkt(crs) -> str | None:
163+
"""Normalise a CRS argument to a WKT string for downstream writers.
164+
165+
Mirrors ``to_geotiff`` / ``write_geotiff_gpu``'s ``crs`` kwarg semantics
166+
so callers can pass an int EPSG code, a WKT string, or a PROJ string
167+
interchangeably. Returns the canonical WKT string (or ``None`` if
168+
``crs`` is ``None``) for forwarding to ``_vrt.write_vrt``, which only
169+
speaks WKT.
170+
171+
Used by ``write_vrt`` (see issue #1715) to close the parameter-naming
172+
drift versus the eager and GPU writer entry points.
173+
174+
Parameters
175+
----------
176+
crs : int, str, or None
177+
EPSG code (int), WKT string, or PROJ string. ``None`` returns
178+
``None`` (the downstream writer falls back to the first source
179+
file's CRS).
180+
181+
Returns
182+
-------
183+
str or None
184+
Canonical WKT string, or ``None`` if ``crs`` is ``None``.
185+
186+
Raises
187+
------
188+
TypeError
189+
If ``crs`` is not an int, str, or ``None``.
190+
ValueError
191+
If ``crs`` is an int that pyproj cannot resolve to a known CRS,
192+
or a string that pyproj cannot parse.
193+
ImportError
194+
If pyproj is not installed and ``crs`` is supplied as something
195+
other than a string. (A string is passed through verbatim so the
196+
WKT-only path keeps working without pyproj.)
197+
"""
198+
if crs is None:
199+
return None
200+
if not isinstance(crs, (int, str)):
201+
raise TypeError(
202+
f"crs must be int (EPSG code), str (WKT or PROJ), or None; "
203+
f"got {type(crs).__name__}")
204+
if isinstance(crs, str):
205+
# Empty string is a common "no CRS" sentinel from upstream
206+
# GeoTIFFs; preserve the existing _vrt.write_vrt semantics (it
207+
# falls back to the first source's CRS for empty strings too).
208+
if not crs:
209+
return None
210+
# If the caller already handed us a WKT, return it untouched.
211+
# PROJCS/GEOGCS/PROJCRS/GEOGCRS are the standard WKT root
212+
# keywords; anything else (EPSG:NNNN, +proj=...) gets normalised
213+
# through pyproj so the downstream XML sees a canonical WKT.
214+
if crs.lstrip().startswith(('PROJCS', 'GEOGCS', 'PROJCRS', 'GEOGCRS',
215+
'COMPD_CS', 'COMPOUNDCRS')):
216+
return crs
217+
try:
218+
from pyproj import CRS
219+
except ImportError as exc:
220+
raise ImportError(
221+
"pyproj is required to convert non-WKT CRS strings (got "
222+
f"{crs!r}). Pass a WKT string directly, or install pyproj."
223+
) from exc
224+
try:
225+
return CRS.from_user_input(crs).to_wkt()
226+
except Exception as exc:
227+
raise ValueError(
228+
f"Could not parse crs={crs!r} as an EPSG/PROJ/WKT string: "
229+
f"{type(exc).__name__}: {exc}"
230+
) from exc
231+
# int branch: convert EPSG -> WKT via pyproj.
232+
try:
233+
from pyproj import CRS
234+
except ImportError as exc:
235+
raise ImportError(
236+
f"pyproj is required to convert crs={crs} (EPSG int) to WKT. "
237+
"Install pyproj, or pass crs as a WKT string."
238+
) from exc
239+
try:
240+
return CRS.from_epsg(crs).to_wkt()
241+
except Exception as exc:
242+
raise ValueError(
243+
f"Could not resolve EPSG:{crs}: {type(exc).__name__}: {exc}"
244+
) from exc
245+
246+
156247
def _geo_to_coords(geo_info, height: int, width: int) -> dict:
157248
"""Build y/x coordinate arrays from GeoInfo.
158249
@@ -3450,7 +3541,8 @@ def _sentinel_for_dtype(nodata_val, dtype):
34503541

34513542
def write_vrt(vrt_path: str, source_files: list[str], *,
34523543
relative: bool = True,
3453-
crs_wkt: str | None = None,
3544+
crs: int | str | None = None,
3545+
crs_wkt: str | None = _CRS_WKT_DEPRECATED_SENTINEL,
34543546
nodata: float | int | None = None) -> str:
34553547
"""Generate a VRT file that mosaics multiple GeoTIFF tiles.
34563548
@@ -3462,9 +3554,17 @@ def write_vrt(vrt_path: str, source_files: list[str], *,
34623554
Paths to the source GeoTIFF files.
34633555
relative : bool, optional
34643556
Store source paths relative to the VRT file (default True).
3465-
crs_wkt : str or None, optional
3466-
CRS as a WKT string. If None, the CRS is taken from the first
3467-
source GeoTIFF.
3557+
crs : int, str, or None, optional
3558+
EPSG code (int), WKT string, or PROJ string. If None, the CRS
3559+
is taken from the first source GeoTIFF. Mirrors the ``crs``
3560+
kwarg on ``to_geotiff`` and ``write_geotiff_gpu`` so the same
3561+
value can be forwarded to whichever writer the caller picked
3562+
without per-writer special-casing (issue #1715).
3563+
crs_wkt : str, optional
3564+
Deprecated alias for ``crs``. Emits ``DeprecationWarning`` when
3565+
used. Passing both ``crs`` and ``crs_wkt`` raises ``TypeError``.
3566+
Accepts a WKT string only (the historic surface); use ``crs``
3567+
for EPSG int or PROJ string input.
34683568
nodata : float, int, or None, optional
34693569
NoData value. If None, taken from the first source GeoTIFF.
34703570
Integer sentinels (e.g. ``65535`` for uint16, ``-9999`` for
@@ -3478,13 +3578,36 @@ def write_vrt(vrt_path: str, source_files: list[str], *,
34783578
"""
34793579
# Explicit signature (previously ``**kwargs``) so ``inspect.signature``,
34803580
# IDE autocomplete, and ``mypy --strict`` can see the accepted kwargs
3481-
# without parsing the docstring. Mirrors ``_vrt.write_vrt`` exactly; if
3482-
# that signature changes, this wrapper must be updated in lockstep.
3581+
# without parsing the docstring. Mirrors ``_vrt.write_vrt`` for the
3582+
# historic ``crs_wkt`` path; the new ``crs`` path normalises through
3583+
# ``_resolve_crs_to_wkt`` before forwarding because the internal
3584+
# writer still only speaks WKT.
3585+
crs_wkt_passed = crs_wkt is not _CRS_WKT_DEPRECATED_SENTINEL
3586+
if crs is not None and crs_wkt_passed:
3587+
# Both supplied is ambiguous regardless of whether the WKT happens
3588+
# to encode the same CRS as the int. Refuse rather than silently
3589+
# picking one.
3590+
raise TypeError(
3591+
"write_vrt: pass either 'crs' or the deprecated 'crs_wkt' "
3592+
"alias, not both.")
3593+
if crs_wkt_passed:
3594+
warnings.warn(
3595+
"write_vrt(..., crs_wkt=...) is deprecated; use crs=... "
3596+
"instead. The kwarg was renamed for parity with to_geotiff "
3597+
"and write_geotiff_gpu, which already accept 'crs' as either "
3598+
"an int EPSG code or a WKT string.",
3599+
DeprecationWarning,
3600+
stacklevel=2,
3601+
)
3602+
crs = crs_wkt
3603+
3604+
resolved_wkt = _resolve_crs_to_wkt(crs)
3605+
34833606
from ._vrt import write_vrt as _write_vrt_internal
34843607
return _write_vrt_internal(
34853608
vrt_path, source_files,
34863609
relative=relative,
3487-
crs_wkt=crs_wkt,
3610+
crs_wkt=resolved_wkt,
34883611
nodata=nodata,
34893612
)
34903613

xrspatial/geotiff/tests/test_kwarg_behaviour_2026_05_12.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,16 @@ def test_relative_false_parses_back_to_same_source(self, source_tif, tmp_path):
204204

205205

206206
class TestWriteVrtCrsWktBehaviour:
207-
"""``crs_wkt=`` overrides the first source's CRS WKT. Without an
208-
override, the first source's WKT is propagated. With an override,
209-
the override wins."""
207+
"""``crs=`` overrides the first source's CRS. Without an override,
208+
the first source's WKT is propagated. With an override, the
209+
override wins.
210+
211+
Pre-#1715 the kwarg was named ``crs_wkt``. The new canonical name
212+
is ``crs`` (parity with ``to_geotiff`` / ``write_geotiff_gpu``);
213+
the old name is still accepted with ``DeprecationWarning``. These
214+
tests exercise the new path; the deprecated path is covered by
215+
``test_write_vrt_crs_1715.py``.
216+
"""
210217

211218
def _read_parsed(self, vrt_path, tmp_path):
212219
with open(vrt_path, 'r') as fh:
@@ -221,7 +228,7 @@ def test_crs_wkt_override_wins(self, source_tif, tmp_path):
221228
'PROJECTION["Transverse_Mercator"],UNIT["metre",1]]'
222229
)
223230
vrt_path = str(tmp_path / 'crs_wkt_override.vrt')
224-
write_vrt(vrt_path, [source_tif], crs_wkt=override)
231+
write_vrt(vrt_path, [source_tif], crs=override)
225232
parsed = self._read_parsed(vrt_path, tmp_path)
226233
assert parsed.crs_wkt == override
227234

@@ -254,7 +261,7 @@ def test_crs_wkt_override_distinct_from_default(self, source_tif, tmp_path):
254261
)
255262
# Override path
256263
vrt_override = str(tmp_path / 'override.vrt')
257-
write_vrt(vrt_override, [source_tif], crs_wkt=override)
264+
write_vrt(vrt_override, [source_tif], crs=override)
258265
# Default path
259266
vrt_default = str(tmp_path / 'default.vrt')
260267
write_vrt(vrt_default, [source_tif])

xrspatial/geotiff/tests/test_polish_1488.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,11 @@ def test_known_kwargs_accepted(self, tmp_path):
8989
a_path = str(tmp_path / 'a_c5_1488.tif')
9090
write(arr, a_path, compression='none')
9191
vrt_path = str(tmp_path / 'mosaic_c5_1488.vrt')
92-
# All three documented kwargs should be accepted.
93-
write_vrt(vrt_path, [a_path], relative=False, crs_wkt=None,
92+
# All four documented kwargs should be accepted. ``crs`` is the
93+
# canonical name as of #1715 (was ``crs_wkt`` pre-rename); pass
94+
# ``crs=None`` instead of the deprecated alias to avoid the
95+
# DeprecationWarning the alias now emits.
96+
write_vrt(vrt_path, [a_path], relative=False, crs=None,
9497
nodata=-9999.0)
9598
assert os.path.exists(vrt_path)
9699

xrspatial/geotiff/tests/test_signature_parity_1631.py

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,21 +51,34 @@ def _gpu_available() -> bool:
5151

5252

5353
def test_write_vrt_signature_exposes_documented_kwargs():
54-
"""``inspect.signature(write_vrt)`` reports the three accepted kwargs.
54+
"""``inspect.signature(write_vrt)`` reports the four accepted kwargs.
5555
5656
Prior to #1631 the public wrapper used ``**kwargs``, so
5757
``inspect.signature`` only saw ``vrt_path`` and ``source_files``.
58+
Issue #1715 added ``crs`` for parity with ``to_geotiff`` /
59+
``write_geotiff_gpu`` while keeping the historic ``crs_wkt`` as a
60+
deprecated alias (sentinel default so the deprecation shim can
61+
tell "user passed nothing" from "user passed crs_wkt=None").
5862
"""
5963
sig = inspect.signature(write_vrt)
6064
params = sig.parameters
6165
assert 'relative' in params
62-
assert 'crs_wkt' in params
66+
assert 'crs' in params # added in #1715
67+
assert 'crs_wkt' in params # deprecated alias
6368
assert 'nodata' in params
64-
# Defaults must match _vrt.write_vrt
6569
assert params['relative'].default is True
66-
assert params['crs_wkt'].default is None
70+
# ``crs`` is the new canonical kwarg; default None means "pick from
71+
# the first source", matching to_geotiff / write_geotiff_gpu.
72+
assert params['crs'].default is None
73+
# ``crs_wkt`` carries a sentinel default so the deprecation shim
74+
# can distinguish "user passed nothing" (no warning) from "user
75+
# passed crs_wkt=None" (deprecated-but-explicit, warn). The
76+
# sentinel itself is private; check that it is NOT None so a
77+
# future maintainer cannot accidentally drop the sentinel logic.
78+
assert params['crs_wkt'].default is not None
79+
assert params['crs_wkt'].default is not inspect.Parameter.empty
6780
assert params['nodata'].default is None
68-
# No more catch-all VAR_KEYWORD
81+
# No catch-all VAR_KEYWORD
6982
kinds = {p.kind for p in params.values()}
7083
assert inspect.Parameter.VAR_KEYWORD not in kinds
7184

@@ -88,7 +101,12 @@ def test_write_vrt_unknown_kwarg_rejected_at_public_level(tmp_path):
88101

89102

90103
def test_write_vrt_accepts_documented_kwargs(tmp_path):
91-
"""Each documented kwarg round-trips through the explicit signature."""
104+
"""Each documented kwarg round-trips through the explicit signature.
105+
106+
Uses the new ``crs=None`` kwarg form (issue #1715). The deprecated
107+
``crs_wkt`` alias is exercised separately in
108+
``test_namespace_no_leak_1715.py``.
109+
"""
92110
arr = np.zeros((8, 8), dtype=np.float32)
93111
da = xr.DataArray(
94112
arr, dims=['y', 'x'],
@@ -101,7 +119,7 @@ def test_write_vrt_accepts_documented_kwargs(tmp_path):
101119
vrt_path = str(tmp_path / 't.vrt')
102120
out = write_vrt(
103121
vrt_path, [tif_path],
104-
relative=False, crs_wkt=None, nodata=-9999.0,
122+
relative=False, crs=None, nodata=-9999.0,
105123
)
106124
assert out == vrt_path
107125
assert os.path.exists(vrt_path)

0 commit comments

Comments
 (0)