Skip to content

feat: addition of multiscale pyramids for cloud/snow probability and classification layers (supersedes #313) - #318

Merged
claxn merged 65 commits into
mainfrom
feat/s2-cld-snw-scl-MS
Oct 27, 2026
Merged

claxn merged 65 commits into
mainfrom
feat/s2-cld-snw-scl-MS

Conversation

@claxn

@claxn claxn commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

This PR supersedes #313 and addresses the latest review comments. After all open questions are solved in #313, it will be closed and the next review round will be done in this PR.

d-v-b and others added 30 commits August 11, 2026 13:49
Adds eopf_geozarr.cpm, an integration package for eopf-cpm (the eopf
package, available via the new 'cpm' extra on Python >= 3.13):

- GeoZarrWriter, registered in CPM's EOWriterRegistry as engine
  "geozarr" (by-name only: cpm_zarr already claims .zarr targets in
  by-target discovery). Routes products to the Sentinel-2 optimized
  pipeline or the generic pipeline based on the declared CPM
  product:type, with a structural fallback. Zarr v3 only; rejects
  unsupported modes, formats, and options loudly.
- an 'eopf convert-geozarr' subcommand exposed through CPM's eopf.cli
  entry-point group, delegating to eopf.store.convert with the engine
  preset, enabling one-pass SAFE -> GeoZarr conversion.
- routing helpers importable without eopf installed.

convert_s2/convert_s2_optimized now skip backing-store validation for
DataTrees with no zarr backend (e.g. built in memory by the CPM SAFE
reader) instead of crashing in get_zarr_group.

Verified against eopf 3.0.0 on Python 3.13: engine registration,
convert-geozarr discovery in the eopf CLI, and an end-to-end
write_datatree(engine="geozarr") producing the flat r10m..r720m
pyramid from an S2 example store.

Assisted-by: ClaudeCode:claude-fable-5
Fixes the confirmed findings from a multi-agent adversarial review:

- write() now runs every argument-derivable check (target type, remote
  scheme, w- existence, pipeline routing, required 'groups') BEFORE the
  destructive mode="w" removal of an existing store. Previously a pure
  options mistake (e.g. missing groups) deleted the user's previous
  product and then failed with nothing written.
- accept compute=True (rejecting only compute=False): CPM's staged-output
  path (staging.py) injects compute=True when a remote Dask client is
  active, so the strict kwargs rejection broke the exact --stage-output
  flow the writer's own error message recommends.
- validate_write_options now applies the target checks (type, remote
  scheme), so CPM's triggering preflight rejects unsupported targets at
  parse time instead of at write time.
- routing: only S02MSIL1C/S02MSIL2A products go to the S2-optimized
  pipeline; other MSI types (S02MSIL0_, S02MSIRAW) lack the reflectance
  pyramid structure and now route generic.
- eopf_geozarr.cpm raises a clear ImportError explaining the Python
  >= 3.13 floor when eopf is missing (pip install 'eopf-geozarr[cpm]'
  silently installs nothing on 3.12).
- _validate_s2_input: cover both branches with tests; also skip
  validation for zarr-v3-backed trees (the S2 input model reads Zarr v2
  only and raised TypeError on CPM 3.x products).

Verified on Python 3.13 with eopf 3.0.0: 42 tests pass, including new
regression tests for the destroy-then-fail ordering and compute
handling.

Assisted-by: ClaudeCode:claude-fable-5
- test-cpm job: installs the cpm extra (eopf pinned via uv.lock) on
  Python 3.13 and runs the CPM writer/routing tests on every push/PR,
  so they no longer silently skip in CI.
- test-cpm-latest job: weekly scheduled run against the newest published
  eopf release to catch upstream contract drift (writer registry,
  staging, CLI entry-point loading) independently of PR failures.
- new test: invoke the real `eopf convert-geozarr` console script via
  subprocess on a synthetic S2 store, covering entry-point discovery,
  click option wiring, and convert()'s reader dispatch through to the
  geozarr engine.

Assisted-by: ClaudeCode:claude-fable-5
PR #212 (Sentinel-3 OLCI GeoZarr export) merged to main after #238 was
branched, leaving select_pipeline unable to reach it: OLCI products fell
into the generic pipeline and failed on the missing 'groups' option.

Adds S3_OLCI_PRODUCT_TYPE_PREFIXES and looks_like_sentinel3_olci to
routing.py, structural fallback included -- oa01_radiance is a data
variable of the measurements node, not a child node, unlike the S2
r{N}m pyramid groups. Wires convert_olci_optimized into the writer with
a new s3_olci_optimized flag and an output_grid option, kept additive
to the existing write() signature (s2_optimized is untouched) since
that's the EOWriter entry point other CPM code calls into.
* ci: run CI on the cpm-geozarr branch

The workflow only triggered on main, so no work on the cpm-geozarr
feature branch ever got checks -- including #247, which is 20 commits
and +29105/-3763 across 23 files. The Samples Service runs this branch
in its September GeoZarr production track, so changes reached a
production pipeline unchecked.

Closes #253

* style: wrap the resolved_spatial_chunk conditional to satisfy ruff format

Introduced by the OLCI routing commit, which renamed the local
`pipeline` variable to `selected_pipeline` and pushed the line past the
limit. It went unnoticed because the branch had no CI -- which is what
this PR fixes.
* init project setup

* adding and harmonising the chunking/sharding functionality across measuremetn and non-measurement groups - allows the contuniation of chunking behaviour from input eopf datatree instead if overwriting by force - scale_offset logic not touched

* addign nan masking for coarsening operation for multiscale calualtation (only for s2_optimising, later extend!) response to issue #246

* addign rechunking after readin to circumvent chunking at read-in (which is hardwired to eopf/store/mappers)

* adding L1C injected bands for mutliscale calculation -> with rechukning of added bands to spatial_chunk size

* readding prior launch script

* implementing changes recommended by claxn and d-v-b, mainly docs, cleanup, simplification

* adressing change requests, improving chunk workflow

* simplifing encoding retrieval for (all and) 1d arrays

* hotfix: adding stac metadata to root level was previously not inlcuded -> copying root metadata to new tree root now

* adding docstrings

* working on the tests, frustrated with test layout

* adding zarr v3 test samples

* change of the data_api tests

* removing test structure which is not adhering to s2 specs, also 'notebook' scripts referenced in tests didnt seem to exist

* changing the s2 zarr v3 structure and all associated tests. Also removing all emojis from output messages

* adapting to new zarr layout, changed function calls, and multiscale calculation changes

* removing emojis

* adding checks to add metadata to arrays if keep_scale_offset=False, modifying checks for s2 structure to align with tests, addig docs

* fixing enum to strenum to allow detection of S2type in simple == comparison

* applying changes to validate pre-commit pyright issues

* addressign requested chagnes for stAc metadata addition adn fill_value fallbacks, removing 'experimental_scale_offset_codec'functionality for encoding2

* small issue of not rechuking in tests

* removing addition of multiscales to stac asset metadata, other small commetns

* adding _FillValue to encoding to allow xarray decode_cf to correctly read zarr store in abscence of eopf-xarray reader, removed large commented section

---------

Co-authored-by: Emmanuel Mathot <emmanuel.mathot@gmail.com>
…ng pr247 (#267)

* adding single /measurement/refectance group to stac as requested during pr247

* adressing comments for crs reuse and proj_attrs_for_crs use

* adding test to incorporate new functionalities and expanding comments
Brings in S1 GRD RTC ingestion (#216), release 0.11.0 (#235) and dependency
bumps (#268, #269). Conflicts: tests/conftest.py keeps the cpm-geozarr v3 S2
fixtures and adds main's S1 RTC fixtures; uv.lock re-locked from main's lock.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in the spatial:bbox pixel-edge fix (#275, fixes #266).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ition of generalised functionality to utils which are not used in other functions
…ozarr) but allows a path to which is needed for rechunking
… due to utils.create_uniform_encoding, Requesting input to check if tests or encoding needs to be adapted (in sight of new scale/offset codec)
…float32 per default if keep_scale_offset=False;
…ate_write_options', removing assert and introducing new pass-through error
…esult_datatree, rechunk_dataset_for_encoding, get_chunking_for_encoding), keeping functions from s2 processing (simple_root_consolidation, stream_write_dataset->stream_write_s2dataset)
…_store_root_geo_metadata() which uses shapely for bounds calculation from geojson geometry
@claxn claxn changed the title Feat/s2 cld snw scl ms feat: addition of multiscale pyramids for cloud/snow probability and classification layers (supersedes #313) Oct 15, 2026

@emmanuelmathot emmanuelmathot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. Please consider to small items

Comment thread src/eopf_geozarr/s2_optimization/s2_multiscale.py Outdated
Comment thread tests/test_s2_multiscale.py Outdated
@claxn

claxn commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

@emmanuelmathot thanks for the quick review - the latest changes were not there yet. I should have addressed now all comments. I also added a new MultiscaleSpec class as a new interface.

@lhoupert I uploaded the latest data (Sentinel-2 only, same files) to s3://continuous-integration/eopf-geozarr/branches/s2-cld-snw-scl-MS. Regarding your 403 issue, this is right now not possible, since we are filtering subdirectories for listing. But we might move it over to a separate bucket, then this issue should be gone.

Thanks!

@lhoupert

Copy link
Copy Markdown
Contributor

Hi @claxn. no further points beyond Emmanuel's two open items.

Acceptance checks work well for L2A T33UWQ. But for L1C T27WXN, the upload looks incomplete, with no root zarr.json and reflectance stopping in r20m. Could it be re-uploaded when you have 5 mins? thanks!

@claxn

claxn commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator Author

Hi @claxn. no further points beyond Emmanuel's two open items.

Acceptance checks work well for L2A T33UWQ. But for L1C T27WXN, the upload looks incomplete, with no root zarr.json and reflectance stopping in r20m. Could it be re-uploaded when you have 5 mins? thanks!

Ah sorry, the upload was interrupted - now it should be complete. I hope now I addressed everything - thanks!

@emmanuelmathot let me know if the test_simple_root_consolidation_aux_multiscales is enough or if you want to explicitly validate consolidation with the actual convert_s2_optimized.

@github-actions

Copy link
Copy Markdown
Contributor

Security Audit Report

View workflow run

Bandit — Static Security Analysis (Security tab)

12 issue(s) found: 1 medium, 11 low

✅ No issues at or above HIGH severity.

1 medium, 11 low issue(s) below threshold not shown in table.

pip-audit — Dependency Vulnerabilities (Security tab)

Dependencies audited: 120, skipped: 0.

Package Version ID Fix Versions Description
filelock 3.32.6 CVE-2026-112940 4.2.1 ReadWriteLock and AsyncReadWriteLock (the SQLite-backed reader/writer lock) hand SQLite a name they do not control.
filelock 3.32.6 CVE-2026-112913 4.2.1 On Windows, ReadWriteLock and AsyncReadWriteLock (the SQLite-backed reader/writer lock) connect SQLite to the lock d
filelock 3.32.6 CVE-2026-112907 4.2.1 On Windows the soft locks (SoftFileLock, MarkerSoftFileLock, SoftFileLease, their async forms, and a FileLock th
virtualenv 21.14.9 CVE-2026-113713 21.14.10 ## Summary The app-data seeder can leave an environment linked to temporary package files that virtualenv deletes when

4 vulnerability/vulnerabilities found (4 fixable) across 2 package(s).


Result: ❌ Blocking issues found — see details above.

@claxn

claxn commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

@lhoupert can I squash and merge this branch from your side?

@lhoupert lhoupert left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @claxn! The _FillValue fix and version stamp look good.

Maybe a follow-up PR could be needed before the backfill: quality/probability/r60m is one 1830×1830 chunk while the other r60m arrays use 2048/1024.

@claxn

claxn commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

@lhoupert thanks for the quick check and the finding - I will fix this in a new PR after this one #314 !

@claxn
claxn merged commit 7696245 into main Oct 27, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants