Open-EO / openeo-python-client

Python client API for OpenEO
https://open-eo.github.io/openeo-python-client/
Apache License 2.0
147 stars 37 forks source link

drop `extra-indices-dict.json` (and provide alternative mechanism for custom indices) #506

Open soxofaan opened 9 months ago

soxofaan commented 9 months ago

the (Awesome) Spectral Indices implementation in openeo had a extra-indices-dict.json file with additional index definitions from the beginning, but @mbuchhorn correctly points out in #501 that this is a design error as it blindly overwrites indices Awesome Spectral Indices with a colliding name (e.g. NDGI, NDMI and S2WI at the moment).

I already removed the colliding indices NDGI, NDMI and S2WI from extra-indices-dict.json in 08cf2aa317a0ca23d1da952ba8139798184161a9

I think we should get rid of extra-indices-dict.json as a whole and think of another mechanism to work with custom index definitions

soxofaan commented 9 months ago

@bontekasper in #271 you added a bunch of custom indices (BI2, BI_B08, S2WI, LSWI_B12) for the Digifed project.

Can you comment on the impact of removing extra-indices-dict.json? Is that a problem for a running project, or is it a non-issue?

Note that I already removed S2WI (titled "Sentinel-2 soil moisture index" in your commit), as it conflicted with "Sentinel-2 Water Index" from Awesome Spectral Indices

mbuchhorn commented 9 months ago

@soxofaan If you remove this additional json with definition of Indices than that it is fine with me, but you have to give us then an alternative to add additional Indices to the calculation routine.

I have several indices like ANIR, NAUC, AUC which should be generated but they are not included in the Awesome Package.

soxofaan commented 9 months ago

An alternative solution for custom indices could be this:

a predefined index from Awesome Spectral Indices is used like

indices = compute_index(cube, index="NDVI")

a custom index could be specified as

indices = compute_index(cube, index="(N - R)/(N + R)")

Maybe it should be made a bit more explicit that the string is a formula and not an abbreviation, e.g.

indices = compute_index(cube, formula="(N - R)/(N + R)")

or

indices = compute_index(cube, index=openeo.formula("(N - R)/(N + R)"))

at first sight it doesn't look that difficult to make something like that possible.

Feedback on these patterns is appreciated

bontekasper commented 9 months ago

@bontekasper in #271 you added a bunch of custom indices (BI2, BI_B08, S2WI, LSWI_B12) for the Digifed project.

Can you comment on the impact of removing extra-indices-dict.json? Is that a problem for a running project, or is it a non-issue?

Note that I already removed S2WI (titled "Sentinel-2 soil moisture index" in your commit), as it conflicted with "Sentinel-2 Water Index" from Awesome Spectral Indices

It will have an impact on the 'EC_zonation_service', but I will try to calculate these indices locally instead of loading them from the spectral dict, which more save for any adaptations in the future I think

soxofaan commented 9 months ago

It will have an impact on the 'EC_zonation_service',

does that service actively use functions from the openeo.extra.spectral_indices package? Or does it just depend on a static process graph (UDP) that was once generated using openeo.extra.spectral_indices?

bontekasper commented 9 months ago

Looking to the process graph of the UDP it seems it used a static version loaded once. However, I adjusted already the code that I can create this indices locally instead of relying on the spectral indices database