Skip to content

Split SUPPORTED_FEATURES into writer.cog / reader.local_cog / reader.http_cog (#2291)#2295

Merged
brendancol merged 2 commits into
xarray-contrib:mainfrom
brendancol:2291-split-cog-supported-features
May 22, 2026
Merged

Split SUPPORTED_FEATURES into writer.cog / reader.local_cog / reader.http_cog (#2291)#2295
brendancol merged 2 commits into
xarray-contrib:mainfrom
brendancol:2291-split-cog-supported-features

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Closes #2291. Part of the #2286 COG readiness rollout.

What changed

  • xrspatial/geotiff/_attrs.py: adds reader.local_cog and reader.http_cog to SUPPORTED_FEATURES at the advanced tier. writer.cog keeps its current tier. The comment block above the dict picks up a short note on why the three keys are separate.
  • xrspatial/geotiff/tests/test_supported_features_tiers_2137.py: small inventory test that asserts the three keys exist and carry a known tier label.

What did not change

Why the split

The three COG code paths have separate stability profiles. The writer emits a self-contained COG layout, the local reader walks overview IFDs on disk, and the HTTP reader sits on top of range-request fetching and partial-IFD handling. Folding them under one key would tie any promotion of one path to a promotion of all three.

Test plan

  • pytest xrspatial/geotiff/tests/ passes locally (5118 passed, 68 skipped).
  • The new inventory assertion fires if any of the three keys is dropped.

…reader keys (xarray-contrib#2291)

Foundational, no behaviour change. Adds reader.local_cog and
reader.http_cog alongside the existing writer.cog so the three COG
code paths can move between tiers on independent tracks. Production
code still gates on the underlying writer / reader options; the keys
are inventory only.

Extends the existing tier inventory test to pin the three keys.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 22, 2026
@brendancol
Copy link
Copy Markdown
Contributor Author

PR Review

Inventory-only change, scoped tightly to what #2291 asks for. Two new reader keys, one updated block comment, one new test. Nothing else touched.

Blockers

None.

Suggestions

None.

Nits

  • xrspatial/geotiff/_attrs.py line 231: the inline comment reads "split from the single writer.cog entry," which makes it sound like the writer key was renamed. It was kept as-is and two reader keys were added next to it. Rewording to "split out from the previous single COG concept" would be less misleading for someone skimming the dict later. Not a blocker.

What looks good

  • Purely additive. No tier promotion, no production code path branches on the new keys, no docs touched (per the issue, docs ride on later PRs in the Promote COG support to ready/stable with compliance and parity gates #2286 chain).
  • The block comment above SUPPORTED_FEATURES records why the three keys are separate, so the rationale is visible without needing git blame.
  • The new test pins both presence and a valid tier label for each of the three keys, so a future refactor that drops one of them fails fast.
  • A grep over xrspatial/ and docs/ for the new key names found no other references, confirming the change cannot collide with a stale call site.
  • The existing test_supported_features_is_a_mapping walks every entry, so the new keys also pick up the generic shape assertion for free.

Checklist

Almost everything in the standard checklist is n/a for an inventory-only change. The two items that do apply both pass:

  • Edge cases covered by tests: presence + tier label asserted for each of the three keys.
  • Docstrings / comments accurate: the block comment now explains the split.

@brendancol
Copy link
Copy Markdown
Contributor Author

Re-review (round 2)

Picked up the one nit from the first pass. The inline comment at xrspatial/geotiff/_attrs.py now reads "split out from the previous single COG concept (which only carried writer.cog)" instead of "split from the single writer.cog entry," so a future reader skimming the dict will not get the impression that the writer key was renamed.

Diff is otherwise unchanged from round 1. No new findings. Marking the review clean from my side.

@brendancol brendancol marked this pull request as ready for review May 22, 2026 02:57
@brendancol brendancol merged commit bb046b2 into xarray-contrib:main May 22, 2026
6 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.

COG readiness: split SUPPORTED_FEATURES into writer.cog / reader.local_cog / reader.http_cog (#2286 PR 1/6)

1 participant