pygeoapi icon indicating copy to clipboard operation
pygeoapi copied to clipboard

datetime parameter not working

Open timtuun opened this issue 6 years ago • 6 comments

http -query with datetime parameter produces exception.

To reproduce make following request: http://localhost:5000/collections/lakes/items?datetime=2018-02-12T23%3A20%3A52Z&f=json

This format should be accepted as it is one of the example formats in the standard: http://docs.opengeospatial.org/is/17-069r3/17-069r3.html#_parameter_datetime (Example 6)

Result is: TypeError: can't compare datetime.datetime to datetime.date

pygeoapi/api.py", line 539, in get_collection_items [Open an interactive python shell in this frame] if datetime__ < te['begin']:

Using latest version of pygeoapi (branch master 28.11.2019)

timtuun avatar Nov 29 '19 08:11 timtuun

Confirmed: the datetime format sent is ok (ISO8601). IMO there is a bug where a parsed datetime parameter is compared in order to check for supported timerange of dataset (NB not all Providers will support datetime yet, to my knowledge only the ElasticSearch Provider):

Code starts at api.py#L511:

        datetime_ = args.get('datetime')
        datetime_invalid = False

        if (datetime_ is not None and
                'temporal' in self.config['datasets'][dataset]['extents']):
            te = self.config['datasets'][dataset]['extents']['temporal']

            if '/' in datetime_:  # envelope
.
.            
            else:  # time instant
                datetime__ = dateparse(datetime_)
                LOGGER.debug('detected time instant')
                if te['begin'] is not None and datetime__ != '..':  <== comparing object to String (?)
                    if datetime__ < te['begin']:
                        datetime_invalid = True
                if te['end'] is not None and datetime__ != '..':
                    if datetime__ > te['end']:
                        datetime_invalid = True

Possibly the variable to check should be datetime_ (String) not datetime__ ? (Notice single/double underscores). Possibly the whole check should be more formal, i.e. comparing datetime Python objects and not strings?

In Python to check a (datetime) range you can use a <= x <= b, e.g.:

>>> import datetime
>>> today = datetime.date.today()
>>> margin = datetime.timedelta(days = 3)

>>> today - margin <= datetime.date(2011, 1, 15) <= today + margin
True

justb4 avatar Nov 29 '19 12:11 justb4

I had a quick look at this and it seemed the bug was more that the configuration was something like:

temporal:
  begins: 2010-01-01
  ends: null

And then internally pygeoapi is attempting to compare the datetime submitted in the URL parameter to a date and this raises an exception. I attempted a quick fix but then ran into trouble comparing a time-zone-aware time (the submitted one) against a time-zone-naive datetime (the 2010-01-01 date, when converted into a datetime, has no assigned timezone... I guess it should be assigned UTC?) I think in general there needs to be better tests with different combinations of date, datetime, and tz before any fix is attempted.

alpha-beta-soup avatar Nov 30 '19 08:11 alpha-beta-soup

@alpha-beta-soup you're right, did not read the error message right. Though I did not understand the comparison to '..' for single datetimestamp. Think a one-liner compare should suffice. Indeed should date-time notation in config be more formal. Idea to use ISO8601 notation and always map to datetime objects?

justb4 avatar Nov 30 '19 12:11 justb4

The problem is that yaml.load() converts strings like "2000-12-12" to datetime.date but "2000-12-12T12:12:12" to datetime.datetime. And date and datetime can't be compared directly. Initial fix attached: date2datetime.patch.txt

ancatfi avatar Dec 18 '19 08:12 ancatfi

On the OGC feature core standards - collections, we have an indication of TRS on the json response. I see on the code

https://github.com/geopython/pygeoapi/blob/6d1dcece0e9a40ef072b0e2b7828636090193c7f/pygeoapi/api.py#L374

that we set TRS, if present on the config file.

IMHO, we could just make it as a fix content on the json response since we don't expect any other option aside from the Gregorian calendar other calendars would require an OGC extension.

@alpha-beta-soup @alpha-beta-soup @ancatfi @timtuun Is the TRS mandatory ? I am trying to sort the problems on this ticket before a implementing datetime on sqlite

jorgejesus avatar Oct 04 '20 16:10 jorgejesus

@tomkralidis refactored the code https://github.com/geopython/pygeoapi/commit/8a65087eefce7da8b2b2f52169cb96dfae8f82a3

jorgejesus avatar Oct 11 '20 14:10 jorgejesus

As per RFC4, this Issue has been inactive for 90 days. In order to manage maintenance burden, it will be automatically closed in 7 days.

github-actions[bot] avatar Mar 10 '24 21:03 github-actions[bot]

As per RFC4, this Issue has been closed due to there being no activity for more than 90 days.

github-actions[bot] avatar Mar 24 '24 03:03 github-actions[bot]