Skip to content
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

Bug in ItemSearch._format_datetime giving wrong date #91

Closed
TomAugspurger opened this issue Aug 31, 2021 · 4 comments
Closed

Bug in ItemSearch._format_datetime giving wrong date #91

TomAugspurger opened this issue Aug 31, 2021 · 4 comments

Comments

@TomAugspurger
Copy link
Collaborator

It looks like _format_datetime ignores components like the seconds in the right side of the range.

>>> import pystac_client
>>> pystac_client.ItemSearch(None)._format_datetime("2019-01-01T00:00:00Z/2019-01-01T00:12:00")
'2019-01-01T00:00:00Z/2019-01-01T23:59:59Z'

I think the expected result is

''2019-01-01T00:00:00Z/2019-01-01T00:12:59Z''
@gadomski
Copy link
Member

gadomski commented Aug 31, 2021

I'm not sure I agree with the expected result, I think it should be exactly what was given but with timezone added, e.g.

2019-01-01T00:00:00Z/2019-01-01T00:12:00Z

The caller could be specifying an exact time down to the seconds value, so we can't assume they wanted the end of that minute, correct? I think the syntax to specify "end of the minute" would be the awkward 2019-01-01T00:00:00Z/2019-01-01T00:12?

@TomAugspurger
Copy link
Collaborator Author

Oh, yeah I think you're right.

Just to confirm some expected behavior, the idea is that if a resolution is not specified, then those will be filled as their highest value. For example, if you only specify up to minutes, then seconds will be filled as 59

>>> assert _format_datetime("2019-01-01T00:00:00Z/2019-01-01T00:12") == "2019-01-01T00:00:00Z/2019-01-01T00:12:59Z"

@gadomski
Copy link
Member

Just to confirm some expected behavior, the idea is that if a resolution is not specified, then those will be filled as their highest value. For example, if you only specify up to minutes, then seconds will be filled as 59

Two corrections:

  1. This is true for the second value (end) in the start/end string, not the first. The first value is filled with the lowest values for unspecified components.
  2. This is only implemented for the date parts of the string, not the time (see Expand partial datetimes when querying #29 for the original request). While it wouldn't be too hard technically, just want to make sure sub-daily support for the "abbreviated" datetime syntax is worth the additional code complexity.

@TomAugspurger
Copy link
Collaborator Author

Gotcha, thanks. I think that https://pystac-client.readthedocs.io/en/latest/client.html#pystac_client.ItemSearch covers that

expands to the end of that day/month/year...

and I can't really think of a better way to phrase it, so sounds good to me.

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