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

dateutil parser breaks search for dates provided in ISO 8601 format by adding timezone offset #330

Closed
torimcd opened this issue Oct 26, 2023 · 3 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@torimcd
Copy link

torimcd commented Oct 26, 2023

Searching with dates in ISO 8601 format (YYYY-MM-DDThh:mm:ssZ) fails due to the dateutil parser adding the timezone offset before sending to the CMR query. The CMR query then fails because the date string is no longer matches the ISO 8601 format that the date was originally in.

Example:

#!/usr/bin/env python 

import earthaccess

COLLECTION_SHORTNAME = 'SPURS2_ARGO'
START_DATE = '2019-03-10T00:00:00Z'
END_DATE = '2019-03-11T23:59:59Z'

auth = earthaccess.login()

cmr_search = earthaccess.DataGranules(auth). \
        short_name(COLLECTION_SHORTNAME).temporal(START_DATE, END_DATE)

results = cmr_search.get()

Fails with the following error:

Traceback (most recent call last):
  File "/lib/python3.11/site-packages/cmr/queries.py", line 357, in convert_to_string
    return date.strftime(iso_8601)
           ^^^^^^^^^^^^^
AttributeError: 'str' object has no attribute 'strftime'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/projects/testearthaccess.py", line 12, in <module>
    short_name(COLLECTION_SHORTNAME).temporal(START_DATE, END_DATE)
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/earthaccess/search.py", line 611, in temporal
    super().temporal(parsed_from, parsed_to, exclude_boundary)
  File "/lib/python3.11/site-packages/cmr/queries.py", line 368, in temporal
    date_from = convert_to_string(date_from)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/lib/python3.11/site-packages/cmr/queries.py", line 361, in convert_to_string
    datetime.strptime(date, iso_8601)
  File "/usr/local/Cellar/python@3.11/3.11.6/Frameworks/Python.framework/Versions/3.11/lib/python3.11/_strptime.py", line 568, in _strptime_datetime
    tt, fraction, gmtoff_fraction = _strptime(data_string, format)
                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/Cellar/python@3.11/3.11.6/Frameworks/Python.framework/Versions/3.11/lib/python3.11/_strptime.py", line 349, in _strptime
    raise ValueError("time data %r does not match format %r" %
ValueError: time data '2019-03-10T00:00:00+00:00Z' does not match format '%Y-%m-%dT%H:%M:%SZ'
@jhkennedy
Copy link
Collaborator

jhkennedy commented Oct 26, 2023

It looks like earthaccess always adds a Z to datestrings, irrespective of whether the datetime that's being formatted has a timezone specified or not:
https://github.com/nsidc/earthaccess/blob/main/earthaccess/search.py#L276

This really should do something like:

      if date_from.tzinfo:
          # eg: '2015-09-25T23:14:42.588601+00:00'
          return date_from.isoformat()
      else:
          # No timezone present - assume UTC.
          # eg: '2015-09-25T23:14:42.588601Z'
          return date_from.isoformat() + 'Z'

Which would be best handled in a time formatting helper function.

But looking at that stacktrace, the cmr package initially tries to convert a datetime object to a string, and if that fails, ensures the string is formatted correctly.

I think earthaccess could just pass datetimes to cmr directly...

@mfisher87 mfisher87 added the bug Something isn't working label Oct 26, 2023
@betolink
Copy link
Member

I like what you're suggesting @jhkennedy! and I'm not sure if this is related to #190

@jhkennedy jhkennedy mentioned this issue Dec 1, 2023
@mfisher87 mfisher87 added the good first issue Good for newcomers label Dec 13, 2023
chuckwondo pushed a commit to chuckwondo/earthaccess that referenced this issue Apr 30, 2024
Bump `python_cmr` version to make use of additional logic in
the `temporal` methods that were pushed from `earthaccess`
up to `python_cmr` itself.

Fixes nsidc#190 nsidc#330
@chuckwondo
Copy link
Collaborator

Fixed by #546

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
Status: Done
5 participants