Skip to content

Align public GeoTIFF docstrings with epic #2340 release contract#2354

Merged
brendancol merged 2 commits into
mainfrom
issue-2346
May 24, 2026
Merged

Align public GeoTIFF docstrings with epic #2340 release contract#2354
brendancol merged 2 commits into
mainfrom
issue-2346

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

  • Adds a consistent [tier] marker (stable / advanced / experimental / internal-only) to every parameter on the seven public GeoTIFF entry points, drawn from epic Epic: GeoTIFF release contract and feature tiering #2340.
  • Adds a short release-contract header to each docstring that points at docs/source/reference/release_gate_geotiff.rst (audit page) and docs/source/reference/geotiff_release_contract.rst (expected path once PR 2 lands).
  • Drops language that implied broader support than the contract: full GDAL VRT parity, warped VRTs, stable GPU paths, stable LERC/J2K/LZ4/JPEG-in-TIFF. Experimental and internal-only paths name their opt-in kwarg inline.

Backend coverage

Docstrings only. No runtime change, so backend coverage of the underlying entry points is unchanged.

Test plan

  • Every public entry point has at least one tier marker and a release-contract reference in its docstring.
  • Existing docstring / contract test suite passes (test_compression_docstring_1644, test_supported_features_tiers_2137, test_release_gate_2321).
  • No doctest examples were broken.

Closes #2346. Part of epic #2340 (PR 3 of 6).

Add a consistent `[tier]` marker (stable / advanced / experimental /
internal-only) to every parameter on the seven public entry points and
fold a short release-contract header into each docstring. Cross-references
point at `docs/source/reference/release_gate_geotiff.rst` (current audit
page) and `docs/source/reference/geotiff_release_contract.rst` (expected
path once PR 2 of the epic lands).

Functions touched (docstrings only, no runtime change):

- open_geotiff
- read_geotiff_dask
- read_geotiff_gpu
- read_vrt (public + internal _vrt.read_vrt)
- to_geotiff
- write_geotiff_gpu
- write_vrt (public + internal _vrt.write_vrt)

Highlights:

- GPU entry points (read_geotiff_gpu, write_geotiff_gpu) now state
  [experimental] up front instead of leaving the caveat in prose.
- LERC / JPEG2000 / J2K / LZ4 are tagged [experimental] and name the
  `allow_experimental_codecs=True` opt-in inline; `compression='jpeg'`
  is tagged [internal-only] and names `allow_internal_only_jpeg=True`.
- VRT entry points name "full GDAL VRT parity" and "warped /
  reprojection VRTs" as out-of-scope rather than leaving the limit
  implicit.
- Internal `_vrt.read_vrt` / `_vrt.write_vrt` are tagged
  [internal-only] so direct callers know they are bypassing the
  dispatcher-level validation.

Part of epic #2340 (PR 3 of 6).
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 24, 2026
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: Align public GeoTIFF docstrings with epic #2340 release contract

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

  • xrspatial/geotiff/__init__.py open_geotiff source param: the inline marker is "[stable for local file paths; advanced for HTTP/fsspec URIs; advanced for .vrt paths]" and does not tier the in-memory file-like buffer case (io.BytesIO). The function accepts that case but it is excluded from the dask / GPU / VRT / remote paths. Name the file-like case explicitly, probably as advanced, since it works but only on the eager numpy path.
  • xrspatial/geotiff/__init__.py around lines 348-360: the bullet list tags JPEG-in-TIFF as [internal-only] and says the read path "only decodes files this library itself wrote." That is the writer contract. The reader can in principle decode any JPEG-in-TIFF, but the round-trip property only holds for files we wrote. Reword so the asymmetry is explicit: write is internal-only, read tolerates external JPEG-in-TIFF on a best-effort basis with no parity claim.
  • xrspatial/geotiff/_backends/gpu.py gpu deprecated alias param: marked [internal-only]. The function-level tier is experimental; marking the deprecated alias as internal-only is defensible (callers should not use it), but experimental may be closer to intent here. The alias is in the public signature and emits a DeprecationWarning rather than refusing.

Nits (optional improvements)

  • xrspatial/geotiff/_writers/eager.py streaming_buffer_bytes: tagged [stable]. The kwarg only matters for dask-backed inputs. Note that explicitly so callers know it is a no-op for numpy.
  • xrspatial/geotiff/_vrt.py read_vrt / write_vrt (the internal symbols): the new [internal-only] header is good. Add an explicit "do not call directly" line at the top so anyone arriving from _backends/vrt.py does not assume the internal symbol carries the same advanced tier.
  • Several params (e.g. name, band on multiple entry points) are tagged [stable]. That is accurate for the param itself, but the function the param lives on is sometimes [experimental] (read_geotiff_gpu). The current convention reads correctly (param tier = function tier), but a one-line note per module that "param-level tiers are bounded by the function-level tier" would head off confusion on the GPU paths.

What looks good

  • Every public entry point has a consistent [tier] marker on every parameter. A small import script confirms all seven functions carry at least one marker, the release-contract docs path, and the epic reference.
  • The "Out of scope for this release" bullet on open_geotiff / to_geotiff names the items the epic calls out (full GDAL VRT parity, warped VRTs, rotated/sheared write support). The release notes can lean on that language directly.
  • The opt-in kwargs are named inline on each experimental / internal-only path (allow_experimental_codecs, allow_internal_only_jpeg, gpu=, chunks=).
  • No runtime change. test_compression_docstring_1644, test_supported_features_tiers_2137, test_release_gate_2321 still pass.

Checklist

  • Algorithm matches reference / paper -- no algorithmic change.
  • All implemented backends produce consistent results -- no runtime change.
  • NaN handling is correct -- no runtime change.
  • Edge cases are covered by tests -- existing coverage applies.
  • Dask chunk boundaries handled correctly -- no runtime change.
  • No premature materialization or unnecessary copies -- no runtime change.
  • Benchmark exists or is not needed -- not applicable.
  • [N/A] README feature matrix updated -- not needed for docstring-only PR.
  • Docstrings present and accurate.

Applies the three suggestions and three nits from the self-review:

- open_geotiff source param: tier the in-memory file-like buffer case
  explicitly as [advanced] (works on the eager numpy path only).
- open_geotiff release-contract header: clarify that JPEG-in-TIFF read
  decodes best-effort with no parity claim, and only the write path is
  internal-only (the encoder omits the JPEGTables tag).
- read_geotiff_gpu deprecated `gpu` alias: change from [internal-only]
  to [experimental] so the tier matches the function-level surface.
- to_geotiff streaming_buffer_bytes: note explicitly that the kwarg is
  a no-op for numpy / CuPy / COG paths.
- _vrt.read_vrt / _vrt.write_vrt: prepend "Do not call this symbol
  directly from external code" so callers arriving from the public
  wrapper do not assume the internal symbol carries the public tier.
- open_geotiff and to_geotiff: add a one-line note that per-parameter
  tier markers are bounded by the function-level surface (heads off
  confusion when a [stable] param appears alongside an [experimental]
  codec or backend choice).
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up review: Align public GeoTIFF docstrings with epic #2340 release contract

All three suggestions and all three nits from the first review pass are applied in commit a89ea66:

  • open_geotiff source param now tiers the io.BytesIO case explicitly as advanced (eager numpy path only).
  • open_geotiff release-contract header now states the JPEG-in-TIFF asymmetry: read is best-effort with no parity claim, write is internal-only.
  • read_geotiff_gpu deprecated gpu alias is now [experimental] to match the function tier.
  • to_geotiff streaming_buffer_bytes notes the no-op behaviour for numpy / CuPy / COG paths.
  • Internal _vrt.read_vrt / _vrt.write_vrt open with "Do not call this symbol directly from external code."
  • open_geotiff and to_geotiff get a one-line note that per-parameter tier markers are bounded by the function-level surface.

Blockers

None.

Suggestions

None remaining.

Nits

None remaining.

Tests

test_compression_docstring_1644, test_supported_features_tiers_2137, test_release_gate_2321 still pass after the follow-up commit (31 passed, 1 xpassed). No runtime change.

Ready for merge once CI is green and a maintainer reviews.

@brendancol brendancol merged commit f10e7fa into main May 24, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Audit public GeoTIFF docstrings against release contract (epic #2340)

1 participant