search

Open-EO / openeo-odc

Functions to map an openEO process graph to a job based on OpenDataCube and Xarray functions.
Apache License 2.0
1 stars 3 forks source link

temporal_extent in load_collection includes the second element #4

Open clausmichele opened 3 years ago

clausmichele commented 3 years ago

The openEO API https://processes.openeo.org/#load_collection specifies that the second element should be excluded.

danielFlemstrom commented 2 years ago

a temporal_extent from 2021-06-01 00:00 to 2021-06-01 23:59 gives no result since of a whole day is subtracted in map_processes_ocs.py::exclusive_date, a few seconds should do the trick (assuming the problem is that OpenEO is "up to" and the cube is "up to and including"). When you have several images a day and want one image, subtracting a whole day makes things difficult for us.

image

Would it be possible to use second instead of day here or would that cause other problems that I cannot see ?

clausmichele commented 2 years ago

Good point, @m-mohr could you please tell me if subtracting 1 second would still be API compliant?

m-mohr commented 2 years ago

I think you may need to subtract depending on the granularity of the input.

If you receive 2020-01-01T00:00:00Z you'd need to subtract a second so that you get 2019-12-31T23:59:59Z. If you receive 2020-01-01 you'd need to subtract a day so that you get 2019-12-31 (at 0:00:00).

danielFlemstrom commented 2 years ago

Should it not be 2019-12-13 (23:59) ?

m-mohr commented 2 years ago

That's a good question. Having a closer look, the process description doesn't define this. Actually, you could probably also just remove 1 s always and be compliant as it's undocumented right now. I think actually my first comment is likely not the common understanding, but the more common understand when people read this is what @danielFlemstrom says. So maybe indeed just remove a second always.

danielFlemstrom commented 2 years ago

Just a clarification of our problem. This is an excerpt of the actual time sequence of a product we have: 2020-10-01 08:20:09 2020-10-01 09:21:39 2020-10-01 09:24:39 2020-10-01 10:01:08 2020-10-01 10:04:08 2020-10-01 11:02:38 2020-10-02 08:55:28 2020-10-02 09:34:57 2020-10-02 09:37:57 2020-10-02 10:34:05 So if you want to look at ONE of these pictures, say 2020-10-01 09:21:39 , it is quite tricky to get it right.

clausmichele commented 2 years ago

If you modify this line https://github.com/Open-EO/openeo-odc/blob/095d9c287d9a98ee77d421c31cb63cb741d42d02/src/openeo_odc/map_processes_odc.py#L60 with return np.datetime_as_string(np.datetime64(date) - np.timedelta64(1, 's'), timezone='UTC') # Substracts one second

and you provide in the openEO process graph a temporal extent of ["2020-10-01T09:21:39Z","2020-10-01T09:22:39Z"] it should work fine!

danielFlemstrom commented 2 years ago

Any chance of getting that fix into the repo ?

clausmichele commented 2 years ago

@danielFlemstrom the project is open source, you are free of creating a fork, modifying the code and open a pull request. Currently this repository is being used in the production back-end of EODC, so I guess they should firstly check that changing this behavior doesn't create problems with unit tests. For sure we will discuss about this, but it's not something we can immediately change.

We are currently quite busy with the preparation of the LPS in Bonn. If you or someone from RISE will take part to the event, you could consider to attend our demos/classrooms or just have a talk with us!