feat: return datetime object for start_time and end_time #127
Conversation
SlowMo24
force-pushed
the
check_datetime2
branch
from
17a6c42 to
621bca9
Compare
| return self._metadata["extractRegion"]["temporalExtent"]["fromTimestamp"] | ||
| return dt.datetime.fromisoformat( | ||
| self._metadata["extractRegion"]["temporalExtent"]["fromTimestamp"].strip( | ||
| "Z" |
There was a problem hiding this comment.
why are we stripping the tz here? if thats what the API gives us, we should happily return it. I think the API should not return a timezoned timestamp but if they do, tant pis
There was a problem hiding this comment.
Since the 'Z' indicates that it is UTC timezone and the ohsome API can handle it with and without it, it is better to not remove it. I don't remember why I stripped it in the previous commented out version. So yes, please do not strip it but return the original ISO timestamp.
There was a problem hiding this comment.
note, that this is not about what the API supports. This is what we present to the user.
But I will adjust it.
There was a problem hiding this comment.
Ok, so I read the documentation and it turns out that python 3.10 does not support it but python 3.11 and 3.12 do.
do we deprecate 3.10 or do we strip the Z?
There was a problem hiding this comment.
switch between the python versions here: https://docs.python.org/3.10/library/datetime.html?highlight=fromisoformat#datetime.datetime.fromisoformat
There was a problem hiding this comment.
Thanks for checking again, I wasn't aware of this. Since 3.10 is still used, I would strip the Z for now, and only remove it once we drop support for it. Maybe add a comment in the code explaining why we strip it.
| return self._metadata["extractRegion"]["temporalExtent"]["toTimestamp"] | ||
| # return dt.datetime.fromisoformat(end_timestamp.strip("Z")) | ||
| return dt.datetime.fromisoformat( | ||
| self._metadata["extractRegion"]["temporalExtent"]["toTimestamp"].strip("Z") |
SlowMo24
changed the title
| return self._metadata["extractRegion"]["temporalExtent"]["fromTimestamp"] | ||
| return dt.datetime.fromisoformat( | ||
| self._metadata["extractRegion"]["temporalExtent"]["fromTimestamp"].strip( | ||
| "Z" |
There was a problem hiding this comment.
Since the 'Z' indicates that it is UTC timezone and the ohsome API can handle it with and without it, it is better to not remove it. I don't remember why I stripped it in the previous commented out version. So yes, please do not strip it but return the original ISO timestamp.
| return self._metadata["extractRegion"]["temporalExtent"]["toTimestamp"] | ||
| # return dt.datetime.fromisoformat(end_timestamp.strip("Z")) | ||
| return dt.datetime.fromisoformat( | ||
| self._metadata["extractRegion"]["temporalExtent"]["toTimestamp"].strip("Z") |
SlowMo24
force-pushed
the
check_datetime2
branch
from
382d9e1 to
a29d20a
Compare
closes #109