gis-ops / routingpy

🌎 Python library to access all public routing, isochrones and matrix APIs in a consistent manner.
https://routingpy.readthedocs.io/en/latest/?badge=latest
Apache License 2.0
252 stars 26 forks source link

Add departure and arrival datetime to Direction #116

Open khamaileon opened 11 months ago

khamaileon commented 11 months ago

For the moment, it works with OTP and Google. I haven't found the corresponding fields in Valhalla. I'd like to improve the unit tests before adding more providers.

nilsnolde commented 11 months ago

Do we really need timezone support for the client? The way we do it in Valhalla is to only accept local time (after all, any snapped graph location‘s timezone is fixed), and we return a tz aware datetime in the response. But of course might be that others put the burden on the client.

nilsnolde commented 11 months ago

I can add support for Valhalla, no worries

khamaileon commented 11 months ago

I see your point. I wanted to be explicit because API input and output formats are often very disparate. For example, OTP takes a local datetime as input and returns a timestamp (UTC). In this particular case, apart from specifying the timezone for the input, I don't see how you can know the local time for the output. By the way, I forgot to specify the timezone for the OTP input.

khamaileon commented 11 months ago

@nilsnolde I committed a "naive" version of the feature. But with hindsight, I think the "aware" version is the best. If you want to use python datetime objects, they have to be timezone aware. Otherwise, it's better to use only UTC timestamps.

nilsnolde commented 11 months ago

Oh yeah @khamaileon, I just realized you're not watching your routingpy fork (obviously, guess you didn't expect a issue/PR there..), but I did open this: https://github.com/khamaileon/routingpy/pull/1. I mentioned it above, but not very explicitly..

khamaileon commented 11 months ago

Oh yeah, I saw it, thanks. I'll try to take a look at it today!