feat: Sentinel-3 OLCI GeoZarr export - #212
Open
d-v-b wants to merge 54 commits into
Open
Conversation
Route multiscales, spatial, and proj convention metadata through a single utils.build_convention_attrs() helper that delegates to zarr_cm.create_many, so zarr-cm validates each convention and emits its CMO. Previously the CMOs were hand-placed and the spatial:/proj:/multiscales keys assembled by hand, which skipped zarr-cm validation (e.g. multiscales' layout>=1 and derived_from=>transform rules) and duplicated key strings. Type the helper precisely with zarr-cm's TypedDicts (SpatialAttrs, GeoProjAttrs, MultiscalesAttrs, MultiConventionAttrs) and a CRSLike Protocol instead of dict[str, Any]. Multiscales data is still produced by the project's MultiscaleMeta model (it also covers the TMS encoding zarr-cm doesn't model), but its CMO and validation now go through zarr-cm. Output is byte-identical to before (verified against the golden-file snapshot tests). Add tests/test_conversion/test_convention_attrs.py covering the helper, including that zarr-cm now rejects an invalid multiscales layout. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ignore root-level scratch files (test.py, tmp.json, cli_test.sh) and the machine-local .claude/ session directory so they stop showing in git status. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This reverts commit 4a5b6e4.
zarr-cm's convention TypedDicts now use PEP 728 `extra_items`, which mypy does not support. Switch the type checker to pyright (which does) and upgrade to the zarr-cm main git dep that also ships the supporting fixes: - Mapping-covariant aggregate API + exported JsonType/JsonValue/JsonDict, so build_convention_attrs no longer needs a private `_core` import or a cast. - PEP 695 `type JsonValue` alias, so pydantic resolves MultiscaleGroupAttrs' ConventionMetadataObject field directly (removed the model_rebuild workaround). Toolchain: - pyproject: replace [tool.mypy] with [tool.pyright]; mypy -> pyright dev dep. - .pre-commit-config / CI lint job: run `uv run --frozen pyright`. Type fixes across src/ and tests/ to reach a clean pyright run (0 errors): - remove stale mypy `# type: ignore[...]` comments (pyright-unnecessary); - narrow NotRequired TypedDict access (`.get()` + guard) instead of making keys Required, preserving runtime validation of optional Sentinel-1/2 members; - replace casts on external/untyped data with runtime isinstance/validation; - model construction via `Model.model_validate(...)` instead of `**dict`. No `typing.Any` introduced. Runtime behavior unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three casts asserted a type derived from Any or a union without verifying it. Replace each with an isinstance guard that raises TypeError on violation, so the assumption is enforced at runtime instead of only asserted to the checker: - sentinel1_reprojection: rio.write_crs() returns Any -> verify xr.Dataset. - s2_multiscale: output_group[base_path] is Array | Group -> verify zarr.Group. - s2_multiscale: client.compute() returns Any -> verify distributed.Future. The remaining casts bridge types on data we already validated (model_dump / create_many output), satisfy protocol/TypeVar binding on `self`, or widen a TypedDict for a third-party API — a runtime check there adds no safety. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
First Sentinel-3 exporter, scoped to OLCI L1 EFR. Grounded in a real EOPF product introspected from the EODC sample store: 21 radiance bands on a single ~300m curvilinear swath grid geolocated by per-pixel 2D lat/lon (no projected CRS). Decision: preserve native swath geometry with CF 2D coordinates (no reprojection); /2 decimation pyramid; measurements-first scope; mirror the S2 package + data_api model + CLI auto-detection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
10-task TDD plan mirroring the S2 exporter: OLCI band mapping, data_api model, swath /2 decimation, structural detection, swath GeoZarr metadata, convert_olci_optimized entry point, CLI auto-detect + convert-s3-olci-optimized, real-product JSON fixture, golden-file snapshot, and verification/docs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pyproject already pins zarr-cm>=0.4.1; update the lockfile from the git-main dev build to the released 0.4.1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…p type) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ion test Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add swath_spatial_attrs() function to return GeoZarr spatial: convention data for curvilinear swath geometry with pixel registration but no affine transform or bbox (geolocation carried by 2-D lat/lon arrays). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… DataTree omits overviews Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add structure-dump fixture (shapes shrunk to 16x16 / tie-points 16x4) from S3A_OL_1_EFR NT product fetched via HTTPS .zmetadata, add s3_olci_group_example conftest fixture, and extend test_s3_olci.py with is_sentinel3_olci_dataset and Sentinel3OlciRoot round-trip tests. Also update Sentinel3OlciMeasurementsMembers to include time_stamp and fix quality/conditions member types to allow nested GroupSpec children. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e time_stamp rows Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a Supported Products section to README.md and a dedicated Sentinel-3 OLCI L1 EFR Conversion section to docs/converter.md, covering: auto-detection via `eopf-geozarr convert`, the dedicated `convert-s3-olci-optimized` command with all key flags, output layout (measurements + r2/r4/... overviews + conditions/quality passthrough), and the v1 scope note (encoding wiring is a follow-up). Also add `# test: skip` to the first code block in the OLCI implementation plan doc so test_docs.py skips the non-runnable planning snippets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s passthrough, fixture dims FIX 0 — reduce_swath: genuine 2×2 fill-aware block-average for radiance vars (OLCI_BANDS), stride-decimate for 2-D coord arrays (lat/lon/alt); replaces literal [::2,::2] decimation of radiance that added no information. FIX 1 — declare overviews as GeoZarr multiscales: build_convention_attrs now receives a MultiscalesAttrs layout (asset "." + "r2"/"r4"/… with relative scale=[2,2] transform) and writes the CMO to measurements attrs. FIX 2 — copy measurements/orphans through (was silently dropped because to_dataset() discards child groups); iterate dt_input["/measurements"].children and _copy_subtree each one. FIX 3 — fixture dim consistency: renamed tie-point 'columns' → 'tie_columns' in geometry/meteorology arrays, fixed orphan 2-D arrays from [16,16] to [16,4] (removed_pixels=4), fixed instrument 2-D arrays to [bands,detectors] at consistent sizes, fixed 3-D meteo arrays to [4,16,4], fixed nb_removed_pixels shape; xr.open_datatree now opens the whole tree cleanly. Snapshot regenerated (conditions, quality, measurements/orphans, r2 overview). FIX 4 — wire in OLCI_BANDS: is_sentinel3_olci_dataset uses OLCI_BANDS[0], reduce_swath identifies radiance vars via OLCI_BANDS frozenset. FIX 5 — remove --enable-sharding / --keep-scale-offset from README and docs/converter.md copy-paste example; keep them in flags table with "accepted but not yet wired" note. All 31 OLCI tests pass; pyright 0 errors; ruff clean; no typing.Any. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…vert tie_columns); regen snapshot Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…egen snapshot - CLI and snapshot test now open OLCI source with mask_and_scale=False so radiance stays uint16 with scale_factor/_FillValue in .attrs; CF decoding no longer silently strips scale/offset or widens dtype to float64. - Add _sanitize_data_vars() in olci_converter.py: strips _eopf_attrs, dtype, valid_min, valid_max from measurement data-var attrs before writing to GeoZarr store (both native and overviews); preserves _FillValue, scale_factor, add_offset, units, standard_name, coordinates. - Update sanitize_array_attrs() in conversion/utils.py: always strip _eopf_attrs/dtype/valid_min/valid_max; only strip _FillValue when is_decoded_float=True (raw-integer path keeps it in attrs for downstream fill handling). - Update _clear_encoding docstring: clarify converter expects raw (non-mask-scaled) input; only Zarr v2 codec encoding is stripped. - Regenerate snapshot: oa01_radiance dtype=uint16, scale_factor/add_offset/ _FillValue present, no _eopf_attrs/dtype/valid_min/valid_max. - Add _assert_radiance_dtype_and_attrs() regression guard in snapshot test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…CI-local attr sanitizer The OLCI work modified the shared sanitize_array_attrs in conversion/utils.py, changing S2/S1/generic output attrs and breaking 3 S2 golden-file snapshot tests. - Restore sanitize_array_attrs to c613e24 original: always strips _eopf_attrs + _FillValue; strips dtype/fill_value/valid_min/valid_max and rewrites digital_counts units only when is_decoded_float=True. - Add _sanitize_olci_array_attrs in olci_converter.py: strips _eopf_attrs/dtype/ valid_min/valid_max while preserving _FillValue (needed for raw uint16 data opened with mask_and_scale=False). - Replace sanitize_array_attrs usage in _sanitize_data_vars with the new helper. - Add unit test asserting _FillValue is preserved and stale keys are stripped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ict detection The fill-collision nudge in reduce_swath could wrap for a sentinel at a dtype bound (e.g. fill 0 on an unsigned dtype nudging to -1 → dtype max). Clamp the nudge candidates so both sides resolve to the in-range neighbour when the sentinel sits at a bound. Document that OLCI auto-detection is intentionally conservative (strict structural validation against Sentinel3OlciRoot): near-miss products fall through to the generic path, and convert-s3-olci-optimized is the explicit bypass. Noted in both detection helpers and the convert subcommand description. Assisted-by: ClaudeCode:claude-fable-5
Working plan/design documents under docs/superpowers/ are local scaffolding, not project documentation; untrack them and gitignore the directory. The files remain on disk (and in history). Assisted-by: ClaudeCode:claude-fable-5
convert_olci_optimized accepts enable_sharding, spatial_chunk, compression_level and keep_scale_offset but does not yet apply them; it now logs a warning when a non-default value is passed, the dedicated subcommand's help marks those flags as not yet applied, and the convert auto-detect path stops forwarding them (it would otherwise warn on every run since convert's defaults differ). In reduce_swath, a float source with a NaN _FillValue was masked via an equality comparison that is a no-op for NaN; mask explicitly with notnull() in that case instead of relying on coarsen's implicit NaN skipping. Assisted-by: ClaudeCode:claude-fable-5
DataTree.from_dict enforces parent/child dimension consistency, so an ancillary group reusing a swath dim name at a different size could make the return-tree assembly fail even though the store was written correctly. Catch that case and fall back to a measurements-only view with a warning instead of masking a successful write. Add direct tests for the reduce_swath fill-collision branch: a valid block averaging exactly to an interior fill sentinel is nudged one step into the valid domain, and an all-fill block keeps the sentinel. (Bound sentinels such as 0/65535 are unreachable by a mean of one-sided valid values; the dtype clamp remains defensive.) Assisted-by: ClaudeCode:claude-fable-5
…ction The radiance branch of reduce_swath evaluated the dask-backed coarsen+mask graph twice (once via isnull().values for the valid mask, again via fill_da.values) — roughly doubling the heaviest compute in the converter for real products. Compute the averaged array once up front. is_sentinel3_olci_dataset only caught ValueError, but from_zarr / model_dump can raise other types on malformed stores; broaden the guard so the classifier never raises for direct callers (the CLI wrapper already swallowed everything). Assisted-by: ClaudeCode:claude-fable-5
…allback Validate up front that the measurements group has the rows/columns swath dimensions the whole OLCI pipeline assumes, raising a clear ValueError instead of a bare KeyError deep in the converter. When return-tree assembly hits a DataTree dimension-consistency conflict, re-add groups greedily and exclude only the offending ones instead of collapsing to a measurements-only view. On the convert auto-detect path, warn when a user's --spatial-chunk or --enable-sharding is dropped (those options are not yet applied by the OLCI converter), instead of ignoring them silently. Assisted-by: ClaudeCode:claude-fable-5
…lback Pass skipna=True explicitly to the coarsen mean so the fill-aware promise (one fill pixel must not contaminate its block) does not hinge on a version-dependent default, and add a test asserting a partially filled block averages only its valid pixels. Sort the return-tree greedy fallback by path depth so parents are considered before their nested children, preventing an implicit empty parent from shadowing the real data-bearing one. Assisted-by: ClaudeCode:claude-fable-5
Truncate any pre-existing store before the first write so re-running convert_olci_optimized against the same output path cannot leave stale overview or ancillary groups from a prior run (the per-group writes were mode="w" scoped to measurements plus mode="a" appends). Tested by converting twice with different min_dimension values. Make swath detection in reduce_swath order-insensitive: a band stored transposed as (columns, rows) is normalized to (rows, columns) and fill-aware block-averaged instead of silently falling through to stride decimation. Document the origin-vs-center offset between decimated overview coordinates and block-averaged radiance in the reduce_swath docstring. Assisted-by: ClaudeCode:claude-fable-5
…book The quadrant visualization nearest-neighbour upsampled every overview back onto the native pixel grid, so nothing in the figure was actually shown at its stored resolution. Replace it with a 2x2 panel figure where each panel pcolormeshes one level's radiance against that level's own decimated 2-D latitude/longitude — same swath coverage per panel, visibly larger native cells per level, no resampling. Levels are picked dynamically from the finest affordable (<~600k quads) to the coarsest. Also make the offline fixture fallback independent of the kernel's working directory (nbconvert starts in docs/notebooks/), and silence consolidated-metadata warnings when opening levels. Assisted-by: ClaudeCode:claude-fable-5
…imated Overview coordinates were stride-decimated, labeling each block-averaged radiance cell with its block's top-left corner pixel — up to half a block (~77 km at r512) away from the centroid of the pixels actually averaged, and never converging to the swath center under repeated reduction. reduce_swath now reduces the 2-D latitude/longitude pair (matched by CF standard_name, falling back to variable name) jointly: pixel positions are mapped to unit vectors on the sphere, block-averaged with fill awareness (a pixel fill in either array is excluded from both), and the mean direction is converted back to degrees. Each overview cell is therefore geolocated at the geodesic centroid of its source block, stable across the antimeridian and at the poles, and collapsing the whole swath to one cell yields its geodesic center. Because the converter runs on un-decoded data and OLCI packs geolocation as int32 microdegrees (scale_factor=1e-06), values are CF-unpacked before the trigonometry and re-packed after, preserving dtype and attrs; feeding raw packed integers into the trig collapses every coordinate to ~(0, 0) (caught on the real EODC product, covered by regression test). Other non-band swath variables (e.g. altitude) keep stride decimation. Notebook re-executed against the real product; prose updated to describe the centroid coordinates. Assisted-by: ClaudeCode:claude-fable-5
Address both findings from roborev job 456: The DataTree returned by convert_olci_optimized was reopened with default CF decoding, handing callers float radiance while the store (and the converter's own input) holds packed uint16. Reopen with mask_and_scale=False and document the raw contract. Overview altitude was stride-decimated, sitting ~half a block from the geodesic-centroid lat/lon locating the same cell. Route altitude (matched by standard_name, falling back to name) through the same fill-aware block mean as radiance: it is linear, so the mean commutes with CF packing and is exact on raw values. Assisted-by: ClaudeCode:claude-fable-5
Short-circuit the opt-out flag first so an explicit --no-s3-olci-optimized skips the structural detection (a full model validation of the store) instead of running it and discarding the result. Roborev job 457, finding 1. Assisted-by: ClaudeCode:claude-fable-5
d-v-b
marked this pull request as ready for review
July 8, 2026 11:30
Contributor
Author
|
this is ready for review @emmanuelmathot |
Contributor
Author
|
a quite explanation of the multiscale coordinates: we downsample using simple windowed averaging on radiances, but to generate the coordinates for the downsampled values, we apply a vector average over the geolocations that contributed to the output location |
…r a map Replace the four-panel figure with a single cartopy PlateCarree map: the swath is split into four geographic quadrants and each quadrant is drawn from a different pyramid level via xarray's curvilinear plot.pcolormesh against that level's own centroid lat/lon, over coastlines, borders, and labeled gridlines with a shared robust color scale. The seamless tiling across quadrant boundaries makes the geodesic-centroid coordinate scheme visible: levels agree on where things are even though every quadrant is a different stored grid. Adds cartopy to the notebooks dependency group. Uses explicit set_xticks/set_yticks with cartopy's Longitude/LatitudeFormatter for axis labels: Gridliner auto-labels (draw_labels=True) blank the whole GeoAxes when a colorbar axes is present under cartopy 0.25 with matplotlib 3.11. Assisted-by: ClaudeCode:claude-fable-5
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 task
Port EOPF-Explorer#220 to the OLCI converter paths: both OLCI source opens (convert routing re-open and convert-s3-olci-optimized) used chunks="auto", which lets dask sub-split stored zarr chunks into multiple read tasks — duplicate egress/decompression and cache races under EOPF-Explorer/data-pipeline#339. open_source_datatree gains a mask_and_scale passthrough since the OLCI converter requires raw packed input. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…port chore: merge main into feat/sentinel3-export
fix(s3-olci): open OLCI sources with native-chunk-aligned reads
Contributor
|
Hello! I ran a real S3A scene through this converter via the staging pipeline (EOPF-Explorer/data-pipeline#370) and reviewed the output store + titiler behaviour against it. Claude identified two gaps that could be a problem for titiler but I let @vincentsarago confirm. 1. Multiscale layout breaks xarray DataTree openingThe converter writes the base-resolution arrays and the 2-D coordinates directly in
|
|
what does |
Contributor
Author
|
great feedback, I will fix the multiscales layout and await guidance for the crs |
Contributor
|
@vincentsarago sorry for the poor wording, I didnt know how to name the transformation from the swath data into xyz tile (the data are in the instrument scan geometry not on a projected grid). LEt me know if I am wrong but to serve xyz tiles we need to resample from the scan geometry into the tile's CRS? Should we do it in the data-model by writing an additional regularly-gridded variant so existing readers dont have to be changed? |
Yeah if the data is not projected to any CRS we won't be able to create the transformation matrix to the input geotransform to the TMS projection |
Contributor
|
Should we do that @d-v-b ? add an regularly-gridded data? which crs do you suggest us to use @vincentsarago EPSG: 4326 ? |
Contributor
Author
Ya I'm happy to reproject. keeping things in instrument coordinates was an indulgence on my part 😄 |
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
4 participants
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.
🤖 AI text below 🤖
Summary
Adds Sentinel-3 OLCI export support, following the same optimized-converter pattern as Sentinel-2:
s3_olci_optimizationpackage:convert_olci_optimizedentry point, fill-aware block-averaged overviews with multiscales convention metadata, swathspatial:*convention attrs (no affine transform for the non-gridded swath geometry), and OLCI band mapping.data_api/s3_olci.py: pydantic-zarr model for the OLCI product layout with round-trip detection.convert, plus an explicitconvert-s3-olci-optimizedsubcommand.sanitize_array_attrskeeps its pre-OLCI semantics); products are opened withmask_and_scale=False; coordinate decimation aligned with coarsen trim on odd dimensions.The branch is up to date with
main(merged after #199 landed).Status
Draft — opening for early visibility and CI.
Test plan
🤖 Generated with Claude Code