Introducing modes of operation by forman · Pull Request #34 · EOPF-Sample-Service/xarray-eopf

forman · 2025-03-19T12:02:12Z

Closes #6

codecov · 2025-03-19T16:40:16Z

Codecov Report

Attention: Patch coverage is 99.57356% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
xarray_eopf/backend.py	97.72%	1 Missing ⚠️
xarray_eopf/spatial.py	98.90%	1 Missing ⚠️

Files with missing lines	Coverage Δ
xarray_eopf/amode.py	`100.00% <100.00%> (ø)`
xarray_eopf/amodes/__init__.py	`100.00% <100.00%> (ø)`
xarray_eopf/amodes/sentinel1.py	`100.00% <100.00%> (ø)`
xarray_eopf/amodes/sentinel2.py	`100.00% <100.00%> (ø)`
xarray_eopf/amodes/sentinel3.py	`100.00% <100.00%> (ø)`
xarray_eopf/constants.py	`100.00% <100.00%> (ø)`
xarray_eopf/filter.py	`100.00% <100.00%> (ø)`
xarray_eopf/flatten.py	`100.00% <100.00%> (ø)`
xarray_eopf/prefix.py	`100.00% <100.00%> (ø)`
xarray_eopf/store.py	`100.00% <100.00%> (ø)`
... and 3 more

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

forman · 2025-04-11T07:07:29Z

allow also Zarr groups like r10m to be opened, thus open_dataset method needs to understand if this is a Zarr group

That might be ok, but may also be achieved by dropping/excluding variables. Anyway it is out of scope of this PR. Let's create an issue and PR for this after this PR.

I am not convinced by the flat structure when using open_dataset in native mode; it looks very complicated, ...

I am, because:

The main intend is to have a view of the complicated data tree as a dataset - no matter how things are named. The naming follows a logical and traceable pattern, that allows for easier access of data variables given well-known keys.
Regarding it looks very complicated: Jupyter notebooks users are not the only ones. For example, you could also use the xarray backend in server context where xarray.Dataset objects are required, not xarray.DataTree and where no user actually looks at the names, hence it doesn't matter whether they "look complicated" or not.

We should add this to the user docs!

We should try to get rid of product_type_name, and extract the product type from the file name; it is possible for a FSMap from fsspec via root; it is possible for zarr.DirectoryStore; I think this should cover most cases.

Yes, we discussed this already once. There is also a TODO in the code and we should create another PR dedicated to this later.

konstntokas

A few general points here and see my comments below. We are almost done!

remove examples/ceph_tests.ipynb
delete cell 22 in examples/open-sen1-native.ipynb and rerun again, the cells after cell 22 are not executed due to the error
[Optional] improve colorbars in examples/open-sen2-analysis.ipynb; set vmin and vmax fo reflectance data; use mask
run isort .`

konstntokas · 2025-04-14T12:32:16Z

@@ -0,0 +1,108 @@
+The xarray backend for EOPF data products `"eopf-zarr"` has two modes of operation,
+namely _analysis mode_ and _native mode_, which are described in the following. 


I think we should write here that the analysis mode is the default.

konstntokas · 2025-04-14T12:42:56Z

+Works basically as `open_datatree()` but using flattened Zarr groups, e.g., groups 
+`r10m`, `r20m`, `r60m` in Sentinel 2 MSI products.
+Flattening includes renaming variables and dimensions by prefixing them using the 
+concatenated names of nested groups. 
+


There is no renaming happening of the variables and coordinates in the analysis mode. I think these 4 lines need some adjustment.

konstntokas · 2025-04-14T14:18:33Z

+OPEN_DT_URL = "https://docs.xarray.dev/en/stable/generated/xarray.open_datatree.html"
+OPEN_DS_URL = "https://docs.xarray.dev/en/stable/generated/xarray.open_dataset.html"
+FSSPEC_USAGE_URL = "https://filesystem-spec.readthedocs.io/en/latest/usage.html"


These constants are not used.

Yes, they are leftovers from my try to use them in f-string docstrings. But unfortunately that didn't work with mkdocs-python.

Removed.

konstntokas · 2025-04-14T14:20:44Z

+        # TODO: also drop now unused coordinates + dimensions as
+        #  they remain even if no longer referenced by any data variables


I created an issue #38

konstntokas · 2025-04-14T15:08:55Z

+                data=rescaled_data,
+                dims=var.dims[:-2] + ref_var.dims[-2:],
+                name=var.name,
+                attrs=var.attrs,


I think we should add information here, if a band was rescaled and what was the original resolution.

Yes, good idea. Will add a history entry.

Co-authored-by: Konstantin Ntokas <38956538+konstntokas@users.noreply.github.com>

forman · 2025-04-14T18:08:43Z

A few general points here and see my comments below. We are almost done!

remove examples/ceph_tests.ipynb

Done.

delete cell 22 in examples/open-sen1-native.ipynb and rerun again, the cells after cell 22 are not executed due to the error

Done.

[Optional] improve colorbars in examples/open-sen2-analysis.ipynb; set vmin and vmax fo reflectance data; use mask

Adjust as you like.

run isort .`

Done.

konstntokas

Thank you, looks all good. We can merge now!

I created issue #38, #39, #40, #41 as follow up tasks.

forman and others added 5 commits March 19, 2025 12:35

Introducing modes

6176f09

introduced mode arg incl validation

bf6fb99

update wrt mode

3f6312e

added Python API

9d294f4

renamed mode into op_mode, op_mode "native" works

e499f50

forman and others added 22 commits March 20, 2025 18:10

Removing common group name prefixes

8106867

Introduced new module "spatial" and "timeit" for performance testing

19089e7

try supporting CEPH bucket naming

61db14f

try supporting CEPH bucket naming (2)

590663f

investigating why there is no chunking still

72e19a3

investigating why there is no chunking still

78cbdad

remove prints

14a69de

chunking works

12310c4

analysis mode works for MSI L1C

c330fa9

analysis mode works for MSI L1C

b9f18d6

sort vars

e51a3fc

Added MSIL2A and params

f67cd30

fix tests

20760ac

added TODOs and (c)

8bcf5ac

added CRS to S2

2aa77df

refactor out assign_grid_mapping(); more TODOs

d524eec

fixed factors

126d0be

generalized resampling

6112102

be less verbose

b59a0ef

using new ProdzctType.get_applicable_params()

47eab1b

added missing TODOs

31e0b70

updated docs

0ca8742

forman changed the title ~~Introducing modes~~ Introducing modes of operation Apr 2, 2025

list all files in bucket

366ed86

forman added 13 commits April 10, 2025 13:04

renamed ProductType --> AnalysisMode

1eb1311

added tests for amodes

c580089

done utils tests

8f292dd

done amode tests

b063cf4

done spatial tests

ef81080

done backend tests

5e40304

added a couple of sentinel2 tests

0773c6b

creating more realistic sentinel-2 dummies

da7e28f

moved test helpers into dedicated subpackage

6fd03b6

sentinel2 tests complete

b551387

store tests complete

e83526e

docstrings

2e6be27

improved backend tests

68bbe4c

forman marked this pull request as ready for review April 11, 2025 07:09

forman requested a review from konstntokas April 11, 2025 07:09

konstntokas requested changes Apr 14, 2025

View reviewed changes

forman and others added 7 commits April 14, 2025 18:17

Update docs/guide.md

132d243

Co-authored-by: Konstantin Ntokas <38956538+konstntokas@users.noreply.github.com>

Update xarray_eopf/flatten.py

485eecb

Co-authored-by: Konstantin Ntokas <38956538+konstntokas@users.noreply.github.com>

updated docs after PR review

61f1350

added history attribute to rescaled variables

67fbd6f

removed print()

dbcdc36

removed unused constants

5aabd76

run isort/ruff

3615d4b

update

756bba5

forman requested a review from konstntokas April 14, 2025 18:11

konstntokas approved these changes Apr 15, 2025

View reviewed changes

forman merged commit 4cdff22 into main Apr 15, 2025
1 of 2 checks passed

forman deleted the forman-6-introduce_modes branch April 16, 2025 07:56

		@@ -0,0 +1,108 @@
		The xarray backend for EOPF data products `"eopf-zarr"` has two modes of operation,
		namely _analysis mode_ and _native mode_, which are described in the following.

		# TODO: also drop now unused coordinates + dimensions as
		# they remain even if no longer referenced by any data variables

Conversation

forman commented Mar 19, 2025

Uh oh!

codecov Bot commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

forman commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

konstntokas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

forman commented Apr 14, 2025

Uh oh!

konstntokas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Mar 19, 2025 •

edited

Loading

forman commented Apr 11, 2025 •

edited

Loading