Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add departure and arrival datetime to Direction #116

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 1 addition & 3 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ matplotlib = {version = "^3.4.1", optional = true}
contextily = {version = "^1.1.0", optional = true}
geopandas = {version = "^0.8.2", optional = true}
descartes = {version = "^1.0.0", optional = true}
pytz = "^2023.3"

[tool.poetry.extras]
notebooks = ["shapely", "ipykernel", "geopandas", "contextily", "matplotlib", "descartes"]
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
certifi==2023.7.22 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
charset-normalizer==3.2.0 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
idna==3.4 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
pytz==2023.3 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
requests==2.31.0 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
urllib3==2.0.4 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
2 changes: 1 addition & 1 deletion requirements_dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pluggy==1.2.0 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
pre-commit==2.21.0 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
pygments==2.15.1 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
pytest==7.4.0 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
pytz==2023.3 ; python_full_version >= "3.8.0" and python_version < "3.9"
pytz==2023.3 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
pyyaml==6.0.1 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
requests==2.31.0 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
responses==0.10.16 ; python_full_version >= "3.8.0" and python_full_version < "4.0.0"
Expand Down
37 changes: 36 additions & 1 deletion routingpy/direction.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"""
:class:`.Direction` returns directions results.
"""
import datetime
from typing import List, Optional


Expand Down Expand Up @@ -65,7 +66,15 @@ class Direction(object):
Contains a parsed directions' response. Access via properties ``geometry``, ``duration`` and ``distance``.
"""

def __init__(self, geometry=None, duration=None, distance=None, raw=None):
def __init__(
self,
geometry: List[List[float]] = None,
duration: int = None,
distance: int = None,
departure_datetime: datetime.datetime = None,
arrival_datetime: datetime.datetime = None,
raw: dict = None,
):
"""
Initialize a :class:`Direction` object to hold the properties of a directions request.

Expand All @@ -78,13 +87,21 @@ def __init__(self, geometry=None, duration=None, distance=None, raw=None):
:param distance: The distance of the direction in meters.
:type distance: int

:param departure_datetime: The departure date and time (timezone aware) of the direction.
:type departure_datetime: datetime.datetime

:param arrival_datetime: The arrival date and time (timezone aware) of the direction.
:type arrival_datetime: datetime.datetime

:param raw: The raw response of an individual direction (for multiple alternative routes) or the whole direction
response.
:type raw: dict
"""
self._geometry = geometry
self._duration = duration
self._distance = distance
self._departure_datetime = departure_datetime
self._arrival_datetime = arrival_datetime
self._raw = raw

@property
Expand Down Expand Up @@ -114,6 +131,24 @@ def distance(self) -> int:
"""
return self._distance

@property
def departure_datetime(self) -> Optional[datetime.datetime]:
"""
The departure date and time (timezone aware) of the direction.

:rtype: datetime.datetime or None
"""
return self._departure_datetime

@property
def arrival_datetime(self) -> Optional[datetime.datetime]:
"""
The arrival date and time (timezone aware) of the direction.

:rtype: datetime.datetime or None
"""
return self._arrival_datetime

@property
def raw(self) -> Optional[dict]:
"""
Expand Down
88 changes: 56 additions & 32 deletions routingpy/routers/google.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@
# the License.
#

import datetime
from operator import itemgetter
from typing import List, Optional, Tuple, Union

import pytz

from .. import convert, utils
from ..client_base import DEFAULT
from ..client_default import Client
Expand Down Expand Up @@ -319,12 +322,40 @@ def directions( # noqa: C901
if transit_routing_preference:
params["transit_routing_preference"] = transit_routing_preference

return self.parse_direction_json(
return self._parse_direction_json(
self.client._request("/directions/json", get_params=params, dry_run=dry_run), alternatives
)

@staticmethod
def parse_direction_json(response, alternatives):
def _time_object_to_aware_datetime(self, time_object):
timestamp = time_object["value"]
dt = datetime.datetime.fromtimestamp(timestamp)
timezone = pytz.timezone(time_object["time_zone"])
return dt.astimezone(timezone)
Comment on lines +329 to +333
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make these new methods util methods instead? I feel it's better to outsource those functions (here and otp) so they can be re-used. I'll do valhalla anyways and will PR to your PR, then you can see if you like that too:)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one in particular is specific to Google because the input format is theirs. Do we output to a Google-specific utility file?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to generalize it a bit in https://github.com/khamaileon/routingpy/pull/1/files


def _parse_legs(self, legs):
duration = 0
distance = 0
geometry = []
departure_datetime = None
arrival_datetime = None

for leg in legs:
duration += leg["duration"]["value"]
distance += leg["distance"]["value"]
for step in leg["steps"]:
geometry.extend(utils.decode_polyline5(step["polyline"]["points"]))

departure_time = legs[0].get("departure_time")
if departure_time:
departure_datetime = self._time_object_to_aware_datetime(departure_time)

arrival_time = legs[-1].get("arrival_time")
if arrival_time:
arrival_datetime = self._time_object_to_aware_datetime(arrival_time)

return duration, distance, geometry, departure_datetime, arrival_datetime

def _parse_direction_json(self, response, alternatives):
if response is None: # pragma: no cover
if alternatives:
return Directions()
Expand All @@ -345,33 +376,27 @@ def parse_direction_json(response, alternatives):

raise error(STATUS_CODES[status]["code"], STATUS_CODES[status]["message"])

if alternatives:
routes = []
for route in response["routes"]:
geometry = []
duration, distance = 0, 0
for leg in route["legs"]:
duration += leg["duration"]["value"]
distance += leg["distance"]["value"]
for step in leg["steps"]:
geometry.extend(utils.decode_polyline5(step["polyline"]["points"]))

routes.append(
Direction(
geometry=geometry, duration=int(duration), distance=int(distance), raw=route
)
directions = []
for route in response["routes"]:
duration, distance, geometry, departure_datetime, arrival_datetime = self._parse_legs(
route["legs"]
)
directions.append(
Direction(
geometry=geometry,
duration=int(duration),
distance=int(distance),
departure_datetime=departure_datetime,
arrival_datetime=arrival_datetime,
raw=route,
)
return Directions(routes, response)
else:
geometry = []
duration, distance = 0, 0
for leg in response["routes"][0]["legs"]:
duration += leg["duration"]["value"]
distance += leg["distance"]["value"]
for step in leg["steps"]:
geometry.extend(utils.decode_polyline5(step["polyline"]["points"]))

return Direction(geometry=geometry, duration=duration, distance=distance, raw=response)
)

if alternatives:
return Directions(directions, raw=response)

elif directions:
return directions[0]

def isochrones(self): # pragma: no cover
raise NotImplementedError
Expand Down Expand Up @@ -506,12 +531,11 @@ def matrix( # noqa: C901
if transit_routing_preference:
params["transit_routing_preference"] = transit_routing_preference

return self.parse_matrix_json(
return self._parse_matrix_json(
self.client._request("/distancematrix/json", get_params=params, dry_run=dry_run)
)

@staticmethod
def parse_matrix_json(response):
def _parse_matrix_json(self, response):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm why this change? this is "public" and staticmethod on purpose, we use this in other projects IIRC

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, it prevented other class methods (_parse_legs) from being called. We'll have to figure out how to rewrite this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a small rewrite, none of those functions needed access to the instance, they can all be static actually

if response is None: # pragma: no cover
return Matrix()

Expand Down
27 changes: 16 additions & 11 deletions routingpy/routers/opentripplanner_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# the License.
#
import datetime
from typing import List, Optional # noqa: F401
from typing import List, Optional

from .. import convert, utils
from ..client_base import DEFAULT
Expand Down Expand Up @@ -167,30 +167,35 @@ def directions(
)
return self._parse_directions_response(response, num_itineraries)

def _timestamp_to_utc_datetime(self, timestamp):
dt = datetime.datetime.fromtimestamp(timestamp / 1000)
return dt.astimezone(datetime.timezone.utc)

def _parse_directions_response(self, response, num_itineraries):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that brings up another good point: this should be static and "public" to be consistent with the other interfaces. we do use this function type in other projects, but also it doesn't need anything from its instance, so it can be static.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same for the isochrones

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one would fit nicely in a utils file because it's generic.

if response is None: # pragma: no cover
return Directions() if num_itineraries > 1 else Direction()

routes = []
directions = []
for itinerary in response["data"]["plan"]["itineraries"]:
geometry, distance = self._parse_legs(itinerary["legs"])
routes.append(
distance, geometry = self._parse_legs(itinerary["legs"])
departure_datetime = self._timestamp_to_utc_datetime(itinerary["startTime"])
arrival_datetime = self._timestamp_to_utc_datetime(itinerary["endTime"])
directions.append(
Direction(
geometry=geometry,
duration=int(itinerary["duration"]),
distance=distance,
departure_datetime=departure_datetime,
arrival_datetime=arrival_datetime,
raw=itinerary,
)
)

if num_itineraries > 1:
return Directions(routes, raw=response)

elif routes:
return routes[0]
return Directions(directions, raw=response)

else:
return Direction()
elif directions:
return directions[0]

def _parse_legs(self, legs):
distance = 0
Expand All @@ -200,7 +205,7 @@ def _parse_legs(self, legs):
geometry.extend(list(reversed(points)))
distance += int(leg["distance"])

return geometry, distance
return distance, geometry

def isochrones(
self,
Expand Down
1 change: 0 additions & 1 deletion tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ def test_skip_api_error(self):
)

client = ClientMock(base_url="https://httpbin.org", skip_api_error=False)
print(client.skip_api_error)
with self.assertRaises(routingpy.exceptions.RouterApiError):
client.directions(url="/post", post_params=self.params)

Expand Down