nilsnolde / 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
272 stars 28 forks source link

Add departure and arrival datetime to Direction #116

Open khamaileon opened 1 year ago

khamaileon commented 1 year 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 1 year 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 1 year ago

I can add support for Valhalla, no worries

khamaileon commented 1 year 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 1 year 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 1 year 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 1 year ago

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