Skip to content

Audit GeoTIFF example notebooks for tier wording (#2345 PR 4, closes #2382)#2385

Merged
brendancol merged 2 commits into
mainfrom
issue-2382
May 25, 2026
Merged

Audit GeoTIFF example notebooks for tier wording (#2345 PR 4, closes #2382)#2385
brendancol merged 2 commits into
mainfrom
issue-2382

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

Adds a tier callout near the top of each GeoTIFF-touching example notebook so a reader can tell which sections sit on the stable contract and which exercise advanced or experimental paths. Each callout names the relevant SUPPORTED_FEATURES key (codec.jpeg2000, reader.http_cog, reader.vrt, etc.) and points to docs/source/reference/geotiff.rst for the full tier matrix. Markdown-only: no code cells, no narrative restructuring, no loaded data changed.

Notebooks touched

  • 25_GLCM_Texture.ipynb: classification section pulls a Sentinel-2 COG over HTTPS (reader.http_cog, advanced).
  • 29_Preview.ipynb: no direct GeoTIFF call, just a see-also pointer.
  • 39_GeoTIFF_IO.ipynb: stable local read/write plus an advanced VRT mosaic section.
  • 40_JPEG2000_Compression.ipynb: experimental codec callout (codec.jpeg2000 / codec.j2k).
  • 46_GeoTIFF_Performance.ipynb: stable dtype / compression_level plus advanced VRT tiled output.
  • 47_Streaming_GeoTIFF_Write.ipynb: stable streaming write plus advanced VRT mosaic.
  • 52_COG_Overview_Generation.ipynb: already had a stable COG callout, added a see-also pointer.

Out of scope

Loaded data, narrative structure beyond the new callouts, and new notebooks.

Test plan

  • All seven notebooks parse as valid nbformat=4 JSON.
  • git diff --stat only touches the seven listed notebooks.
  • Notebooks aren't executed in CI on this repo (no nbval / nbmake / ipynb references in .github/workflows/), so end-to-end execution is unchanged by markdown-only edits.

Closes #2382. Part of epic #2345 (PR 4 of the audit slice).

Adds a tier callout near the top of each GeoTIFF-touching example
notebook so a reader can tell which sections sit on the stable
contract and which exercise advanced or experimental paths.

Each callout names the relevant SUPPORTED_FEATURES key (for example
codec.jpeg2000, reader.http_cog, reader.vrt) and points to
docs/source/reference/geotiff.rst for the full tier matrix.

Notebooks touched: 25_GLCM_Texture, 29_Preview, 39_GeoTIFF_IO,
40_JPEG2000_Compression, 46_GeoTIFF_Performance,
47_Streaming_GeoTIFF_Write, 52_COG_Overview_Generation.

Markdown-only changes. No code cells, narrative ordering, or loaded
data were modified.

Closes #2382. Part of epic #2345.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 25, 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: GeoTIFF example notebook tier audit (#2382)

Blockers

None. Markdown-only audit PR, no code path to break.

Suggestions

  • examples/user_guide/46_GeoTIFF_Performance.ipynb: the new tier-note says the VRT tiled output "sits at the advanced tier (reader.vrt)". The notebook is writing a VRT -- the per-tile write itself rides on writer.local_file (stable); reader.vrt (advanced) only applies on the round-trip read. Same applies to 47_Streaming_GeoTIFF_Write.ipynb. Worth tightening so a reader doesn't conclude the write side is advanced.
  • examples/user_guide/39_GeoTIFF_IO.ipynb: "dask reads / writes are tagged stable" is slightly off. reader.dask is the explicit stable key. There is no writer.dask key in SUPPORTED_FEATURES -- dask writes are covered under writer.local_file (stable). Suggest: "dask reads (reader.dask) and dask streaming writes (writer.local_file) are tagged stable."
  • examples/user_guide/40_JPEG2000_Compression.ipynb: the callout says the GPU path "depends on nvJPEG2000 being available at import time". Worth confirming -- if the nvJPEG2000 binding is loaded lazily on first GPU call, "at call time" is more accurate. Minor.

Nits

  • Six of the seven notebooks ship the same See-also paragraph verbatim. 52 ends up with two pointers to the same reference page (its existing "See the Stable COG contract section..." line and the new See-also). Dropping the duplicate on 52 would be cleaner.
  • examples/user_guide/29_Preview.ipynb: the see-also is fine but could be one sentence -- the opening paragraph of the existing notebook already mentions GeoTIFF, so the long lead-in is more than needed.

What looks good

  • Tier vocabulary matches SUPPORTED_FEATURES and geotiff.rst token-for-token, so a reader can grep across notebook, docstring, and reference doc.
  • The 40_JPEG2000 callout correctly flags both codec.jpeg2000 / codec.j2k and reader.gpu / writer.gpu as experimental, and reminds the reader of allow_experimental_codecs=True.
  • 25_GLCM_Texture isolates the one advanced path in the notebook (HTTP COG) and gives a concrete workaround ("swap the URL for a local path").
  • No notebook claims GDAL / VRT / GPU / codec parity. Acceptance criteria met.
  • JSON parses cleanly. Diff is markdown-only.

Checklist

  • Algorithm / NaN / edge cases / dask boundaries: N/A (no code).
  • Benchmark, README matrix, docstrings: N/A.
  • Tier wording vs SUPPORTED_FEATURES: verified per notebook.
  • No "stable" claim where SUPPORTED_FEATURES says otherwise: confirmed.

…2382)

Review pass on PR #2385:

- 39: separate 'stable local read/write' from the dask claim; reader.dask
  is the explicit stable key, dask writes ride on writer.local_file.
- 40: nvJPEG2000 is loaded lazily on the first GPU call, not at import
  time -- update wording and name the loader-path failure mode.
- 46 / 47: separate the per-tile write (writer.local_file, stable) from
  the VRT round-trip read (reader.vrt, advanced) so a reader does not
  conclude the write side itself is advanced.
- 29: trim the see-also to one sentence.
- 52: drop the duplicate See-also we appended -- the existing 'See the
  Stable COG contract section' line already points at the same page.
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 (loop 2)

Self-review after commit 89a6d5c. Original findings + disposition:

Suggestions (all fixed)

  • 46/47 VRT write/read distinction: fixed. Callouts now state the per-tile write rides on writer.local_file (stable) and only the round-trip read uses reader.vrt (advanced).
  • 39 dask wording: fixed. Split into "stable local read/write" plus "dask reads (reader.dask) and dask streaming writes (covered by writer.local_file)".
  • 40 nvJPEG2000 import-time claim: fixed. Verified against xrspatial/geotiff/_gpu_decode.py _get_nvjpeg2k() -- nvJPEG2000 is loaded lazily on the first GPU call (cached after), not at module import. Callout now says "probes for nvJPEG2000 lazily on the first GPU call" and names the libnvjpeg2k.so loader-path failure mode.

Nits (all fixed)

  • 52 duplicate See-also: fixed. The new See-also paragraph we appended has been removed; the existing "See the Stable COG contract section" line in cell 1 is the single pointer.
  • 29 long see-also: fixed. Trimmed to one sentence.
  • Repeated See-also boilerplate on the other notebooks: kept. The reviewer flagged this as noisy but it's the single canonical pointer to the reference page on each notebook in scope, which is the asked-for "see also pointer at the top" in the issue acceptance criteria.

Remaining state

No open Blockers, Suggestions, or Nits. Closing the loop.

@brendancol brendancol merged commit f8cfc3d into main May 25, 2026
4 of 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 GeoTIFF example notebooks for unsupported-as-stable wording (#2345 PR 4)

1 participant