Open-EO / openeo-processes

Interoperable processes for openEO's big Earth observation cloud processing.
https://processes.openeo.org
Apache License 2.0
49 stars 14 forks source link

`date_between`: inconsistent with `filter_temporal` half-openess #500

Open soxofaan opened 3 months ago

soxofaan commented 3 months ago

This confusion came up in a discussion with @VictorVerhaert: date_between handles the temporal extent as left+right closed by default:

date_between(string x, string min, string max, ?boolean exclude_max = false) : boolean|null

By default, this process checks whether x is later than or equal to min and before or equal to max.

This is not in line with the left-closed definition of the temporal extents in filter_temporal, load_collection, load_stac, ...

soxofaan commented 3 months ago

A naive fix is flipping the default of exclude_max to true, but this is of course a breaking change.

This is an experimental process, but we have indication that it is already being used. For example it is being used in https://github.com/Open-EO/openeo-community-examples/blob/main/python/ParcelDelineation/Parcel%20delineation.ipynb

jdries commented 3 months ago

It's indeed used by 2 or 3 users, but my feeling is that they are not necessarily affected, so I would consider to just fix it.

m-mohr commented 1 month ago

This is aligned with the between process, which is also defaulting to inclusive bounds and has the same signature, including the exclude_max parameter. Does this really need a fix? It can be changed at any time through the parameter.

soxofaan commented 1 month ago

another option:

leave as-is:

date_between(string x, string min, string max, ?boolean exclude_max = false) :  boolean|null

but add

date_within(string x, temporal-interval:array<string|null> extent): boolean|null

Where the interval is handled left-closed (per definition of the temporal-interval subtype), as in all the other places (load_collection, filter_temporal, load_stac). Counter-argument is that this might be confusing, but it should be straightforward to catch/report issues client side as date_between requires at least 3 arguments while date_within requires exactly 2 arguments