geopython / pygeoapi

pygeoapi is a Python server implementation of the OGC API suite of standards. The project emerged as part of the next generation OGC API efforts in 2018 and provides the capability for organizations to deploy a RESTful OGC API endpoint using OpenAPI, GeoJSON, and HTML. pygeoapi is open source and released under an MIT license.
https://pygeoapi.io
MIT License
493 stars 261 forks source link

datetime parameter not working #316

Closed timtuun closed 7 months ago

timtuun commented 4 years ago

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)

justb4 commented 4 years ago

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
alpha-beta-soup commented 4 years ago

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.

justb4 commented 4 years ago

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

ancatfi commented 4 years ago

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

jorgejesus commented 4 years ago

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 commented 4 years ago

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

github-actions[bot] commented 7 months ago

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] commented 7 months ago

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