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
251 stars 26 forks source link

GraphHopper "profile" is set under "vehicle" #93

Closed Serphentas closed 1 year ago

Serphentas commented 1 year ago

Here's what I did

A simple query, such as:

client.directions(locations=[...],[...]],instructions=False,elevation=True,profile="car",**{"avoid":"motorway;ferry"})

Here's what I got

No "profile" set, but a "vehicle" that GraphHopper doesn't accept:

    raise exceptions.RouterApiError(status_code, body)
routingpy.exceptions.RouterApiError: 400 ({'message': 'profile parameter required', 'hints': [{'message': 'profile parameter required', 'details': 'java.lang.IllegalArgumentException'}]})

Here's what I was expecting

The "profile" key to be set accordingly


Here's what I think could be improved

This line should be changed from

params = {"vehicle": profile}

to

params = {"profile": profile}

since no "vehicle" key exists in the GraphHopper API documentation.

nilsnolde commented 1 year ago

Huh.. we just had a PR fixing some other GH things, I can’t imagine that didn’t work a few weeks back. Did they release another major version and changed it?

nilsnolde commented 1 year ago

Jep, GH 7.0 removed the vehicle parameter: https://github.com/graphhopper/graphhopper/blob/master/CHANGELOG.md#70-14-mar-2023

Serphentas commented 1 year ago

Good catch. Shall I go ahead with a PR ? I'm thinking about a version parameter for the client initialization, or perhaps just the directions method, to switch between vehicle and profile.

nilsnolde commented 1 year ago

Shall I go ahead with a PR

that’d be great thanks!

I'm thinking about a version parameter for the client initialization

That’d be noble:) But I think we should just support whatever’s on their public API, since that’s a slippery slope. GH has twice a year major version upgrades currently, too much of a maintenance hassle to support all their deprecations to be honest.

Serphentas commented 1 year ago

See above, I followed your advice and simply replaced the key with no backwards support.

nilsnolde commented 1 year ago

Done in #94