Skip to content

temporal_extent in load_collection includes the second element #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
clausmichele opened this issue May 27, 2021 · 9 comments
Open

Comments

@clausmichele
Copy link
Member

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

@danielFlemstrom
Copy link

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
Copy link
Member Author

clausmichele commented May 12, 2022

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

@m-mohr
Copy link
Member

m-mohr commented May 12, 2022

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
Copy link

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

@m-mohr
Copy link
Member

m-mohr commented May 12, 2022

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
Copy link

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
Copy link
Member Author

If you modify this line

return np.datetime_as_string(np.datetime64(date) - np.timedelta64(1, 'D'), timezone='UTC') # Substracts one day
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
Copy link

Any chance of getting that fix into the repo ?

@clausmichele
Copy link
Member Author

@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!

@m-mohr m-mohr reopened this May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants