[Review only] s1-tiling rebased onto main - #213
Closed
lhoupert wants to merge 15 commits into
Closed
Conversation
…a GeoTIFF→GeoZarr V3 prototype - Implementation plan with Zarr V3 store structure, conventions, and phased roadmap - S1Tiling Docker instructions with EODAG 4.0.0 patch (5 breaking changes fixed) - GeoTIFF inspector script for S1Tiling output validation - Synthetic prototype validating sharding, resize/append, overview generation - Real-data conversion: 3 acquisitions (31TCH) → 1.8 GB Zarr store with 6 overview levels - Key findings: chunk divisibility constraint, datetime format, coverage variability
- Update §2 array metadata example to actual zarr-python 3.1.1 on-disk format
(sharding codec in codecs array, chunk_grid holds shard shape, inner chunks 366)
- Update conditions group: per-orbit naming (gamma_area_{orbit}), mark LIA and
incidence_angle as aspirational (not confirmed as S1Tiling outputs)
- Add Operational Findings subsection: EODAG 4.0.0 patch, DEM tile extent,
GAMMA_AREA filename format
- Fix docker instructions: CALIBRATION tag is GammaNaughtRTC (not gamma_naught_rtc)
- Mark Phase 0 checklist fully complete
- Update §9 API corrections to reflect plan is now corrected
- Add src/eopf_geozarr/data_api/s1_rtc.py — Zarr V3 Pydantic models for S1 GRD γ0T RTC GeoZarr stores, using pyz.v3 GroupSpec/ArraySpec with TypedDict members (same pattern as s2.py uses pyz.v2) - Models: S1RtcRoot, S1RtcOrbitGroup, S1RtcNativeResolutionDataset, S1RtcOverviewResolutionDataset, S1RtcConditionsGroup - Validation: convention UUIDs, spatial:dimensions, multiscales layout, required data arrays (vv/vh/border_mask), gamma_area presence - Add tests/_test_data/s1_rtc_examples/s1-grd-rtc-31TCH.json — realistic fixture with 3 timesteps, 6 overview levels, 3 gamma_area conditions - Add tests/test_data_api/test_s1_rtc.py — 11 tests: round-trip, structure validation, negative cases (missing orbit, r10m, UUIDs, etc.) - Add conftest fixture s1_rtc_json_example parametrized over all fixtures
…es in S1 GRD RTC prototype
…dinate arrays and enforce lowercase dimension names
Productionise the GeoTIFF → Zarr V3 ingestion pipeline for S1Tiling gamma-naught RTC outputs. New module: src/eopf_geozarr/conversion/s1_ingest.py (~500 lines) - extract_geotiff_metadata(): rasterio-based metadata extraction with tag validation and S1Tiling datetime normalisation - create_s1_store(): Zarr V3 store creation with full GeoZarr conventions (multiscales, proj:, spatial:), sharded arrays, 1D spatial coordinate arrays at every resolution level - ingest_s1tiling_acquisition(): main API — create-or-open store, read GeoTIFFs, generate overviews (average/nearest), write all levels, append coordinate variables. CRS/shape consistency validation on append. - consolidate_s1_store(): post-batch metadata consolidation - discover_s1tiling_acquisitions(): file discovery with grouping Tests: tests/test_s1_rtc_ingest.py (27 tests) - Metadata extraction, store creation, ingestion (create + append), data integrity, xarray roundtrip, CRS/shape mismatch rejection, consolidation, file discovery Uses zarr_cm CMO dicts for convention metadata (not hardcoded UUIDs). Reuses calculate_aligned_chunk_size() from utils.py. Exports wired in conversion/__init__.py. Refs: #139
- Replace dict[str, Any] multiscales field with zcm.Multiscales import - Remove inline MultiscalesTransform/ScaleLevel/Multiscales classes - Update spatial:dimensions validator to lowercase ['y', 'x'] (Python 3.12) - Remove typing_extensions import (native in Python 3.12) - Update test assertions for Pydantic model attribute access - Auto-format fixes from ruff (imports, fixture parens, line length)
- Add [tool.ruff.lint.flake8-type-checking] runtime-evaluated-base-classes for pydantic.BaseModel so Pydantic field type imports aren't flagged - Remove 4 stale noqa comments auto-fixed by ruff
…tamp (#183) (#184) discover_s1tiling_acquisitions returned 0 acquisitions for multi-frame S1Tiling products: their GeoTIFF filenames mask the acquisition time as '…txxxxxx', which the filename regex (\d{8}t\d{6}) rejected, so parse_s1tiling_filename returned None and every file was skipped — silently, with no error. - Relax the acq_stamp pattern to accept a masked time (\d{6}|x{6}), so masked files are no longer dropped. - In discover, when the stamp is masked, resolve the real YYYYMMDDtHHMMSS stamp from the GeoTIFF ACQUISITION_DATETIME tag (via extract_geotiff_metadata) and use it for grouping + downstream STAC datetime. This is the durable upstream fix for the data-pipeline #237 workaround (which renamed masked products before discovery); that workaround can now be retired. Closes #183. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The S1 branch's [tool.ruff.lint.flake8-type-checking] runtime-evaluated-base-classes config makes ruff no longer flag TC001 for the pydantic-runtime ProjJSON import in store.py, so main's # noqa: TC001 became unused (RUF100). Remove it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
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
2 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.
What this is
s1-tilingrebased cleanly onto the currentmain(which now includes #199 — zarr-cm 0.4.1 + the mypy→pyright migration).What to review
The diff vs
mainis the whole S1-tiling feature. The only new authored content from the rebase (beyond replaying the existing S1 commits) is:pyproject.toml: kept the S1[tool.ruff.lint.flake8-type-checking]section and main's[tool.pyright]; dropped the old[tool.mypy]config.chore: drop noqa made unnecessary by ruff TC config(main's# noqa: TC001instore.pybecame unused under the S1 ruff config).Verification
uv run --frozen pyrighton the base is clean.