Privatize build_vrt; route to_geotiff's VRT path through it#2979
Merged
Conversation
…h it (#2974) build_vrt is dropped from the public API (__all__, package docstring) and renamed to _build_vrt. to_geotiff's .vrt write path (_write_vrt_tiled) now routes through _build_vrt instead of calling _vrt.write_vrt directly, so _build_vrt is the single internal VRT-index entry point. Tests updated to import the private name; the public-surface assertions now expect only open_geotiff and to_geotiff.
Remove the public build_vrt entry from the GeoTIFF reference, safe-IO user guide, release-gate contract table, README feature matrix, and the internal-dev notes. The user-guide notebook's VRT section now writes a mosaic through to_geotiff's .vrt path (chunked DataArray -> tiled GeoTIFFs + index) instead of calling the removed public helper; its output cell was regenerated from a real run.
brendancol
commented
Jun 5, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
PR Review: Privatize build_vrt; route to_geotiff's VRT path through it
Blockers
None.
Suggestions
None blocking.
Nits
_build_vrt's docstring opening now reads[internal-only], but the five per-parameter tier tags below it still say[advanced](_writers/vrt.py:58-90). Minor internal inconsistency now that the function is no longer a public entry point; worth normalizing to[internal-only].
What looks good
- The routing change is behavior-preserving.
_build_vrt(vrt_path, tile_paths, relative=True, nodata=nodata)resolves towrite_vrt(vrt_path, tile_paths, relative=True, crs_wkt=None, nodata=nodata), the same call the path made before. No deprecation warning fires (pathpositional,crsunset), and the local import sidesteps a circular dependency. - Public-surface assertions are flipped correctly, and the new
test_build_vrt_is_not_publiccovers both directions: the public name is gone and_build_vrtis still importable. - The pre-existing local helper
_build_vrt(tmp_path)intest_finalization.pywould have collided with the imported_build_vrtafter the rename; it was renamed to_make_one_source_vrt, and the imported call inside it was left intact. - The notebook's VRT output cell was regenerated from a real run rather than hand-written, so the printed tile sizes and round-trip result are accurate.
Checklist
- No algorithm change (API-surface refactor)
- Backends unaffected; VRT write still runs eager + dask
- No NaN / edge-case concerns
- Routing change verified behavior-preserving
- No premature materialization or extra copies
- Benchmark not applicable
- README feature matrix updated
- Docstrings present (one tier-tag nit above)
brendancol
commented
Jun 5, 2026
Contributor
Author
brendancol
left a comment
There was a problem hiding this comment.
Follow-up review
The one nit from the first pass is fixed in d04a252: the five per-parameter tier tags in _build_vrt's docstring now read [internal-only], matching the function summary. test_polish.py::TestC5WriteVrtKwargs still passes (the docstring keeps relative / crs_wkt / nodata).
No new findings. The change is docstring-only; no code paths moved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #2974.
build_vrtfrom the public GeoTIFF API and renames the wrapper to_build_vrt(out of__all__and the package docstring).to_geotiff's.vrtwrite path (_write_vrt_tiled) through_build_vrt, so it's the single internal entry point for VRT-index emission. That path used to call_vrt.write_vrtdirectly, which left the publicbuild_vrtwith no internal caller at all.to_geotiff('.vrt')instead of the removed helper._build_vrt; the public-surface assertions now expect onlyopen_geotiffandto_geotiff.build_vrtis advanced-tier, not stable, so it's allowed to change between releases. Anyone who calledxrspatial.geotiff.build_vrteither writes to a.vrtpath throughto_geotiffor imports_build_vrt.Backend coverage
No behavior change. VRT writing already ran on the eager and dask paths; this is an API-surface refactor. Round-trip checked:
to_geotiff(da_chunked, 'mosaic.vrt')tiles the array andopen_geotiffreads it back matching the original.Test plan
{open_geotiff, to_geotiff}, plus a new test thatbuild_vrtis not importable while_build_vrtis_build_vrtto_geotiff('.vrt')passes on numpy and dask