preference parameter isn't read when asking for isochrones with Valhalla engine

[mthh]

Here's what I did and what I got

Asking for isochrones with default values

isochrones = api.isochrones(
    locations=(5.86659,45.26936),
    profile='bicycle',
    interval_type='time',
    intervals=[i * 60 for i in (20,50,80,110)],
)

and asking for isochrones using the preference attribute (but no options attribute)

isochrones_shortest = api.isochrones(
    locations=(5.86659,45.26936),
    profile='bicycle',
    interval_type='time',
    intervals=[i * 60 for i in (20,50,80,110)],
    preference="shortest",
)

both return the same result.
By adding dry_run=True we can see that the preference is not used :

url:
http://localhost:8002/isochrone
Parameters:
{
  "headers": {
    "User-Agent": "routingpy/v1.3.0",
    "Content-Type": "application/json"
  },
  "timeout": 60,
  "json": {
    "locations": [
      {
        "lon": 5.86659,
        "lat": 45.26936
      }
    ],
    "costing": "bicycle",
    "contours": [
      {
        "time": 20.0
      },
      {
        "time": 50.0
      },
      {
        "time": 80.0
      },
      {
        "time": 110.0
      }
    ]
  }
}

However, if you add any values in options, the preference is taken into account :

isochrones_shortest = api.isochrones(
    locations=(5.86659,45.26936),
    profile='bicycle',
    interval_type='time',
    intervals=[i * 60 for i in (20,50,80,110)],
    preference="shortest",
    options={"foo": 12},
    dry_run=True,
)

Output :

url:
http://localhost:8002/isochrone
Parameters:
{
  "headers": {
    "User-Agent": "routingpy/v1.3.0",
    "Content-Type": "application/json"
  },
  "timeout": 60,
  "json": {
    "locations": [
      {
        "lon": 5.86659,
        "lat": 45.26936
      }
    ],
    "costing": "bicycle",
    "contours": [
      {
        "time": 20.0
      },
      {
        "time": 50.0
      },
      {
        "time": 80.0
      },
      {
        "time": 110.0
      }
    ],
    "costing_options": {
      "bicycle": {
        "foo": 12,
        "shortest": true
      }
    }
  }
}

---

Here's what I was expecting

I was expecting the preference parameter to be taken into account, as per the documentation, even if options is None.

---

Here's what I think could be improved

After looking at the code and according to my understanding, in isochrones with Valhalla engine, the preference parameter is only read if the options parameter is not None. See https://github.com/gis-ops/routingpy/blob/e5eb103bac330cefdb54aae23deb30f408815911/routingpy/routers/valhalla.py#L441

A possible fix could be to do as with directions, and replace if options by if options or preference on line 441 ; cf. https://github.com/gis-ops/routingpy/blob/e5eb103bac330cefdb54aae23deb30f408815911/routingpy/routers/valhalla.py#L232

[nilsnolde]

Oops, yeah you're absolutely right. Do you time for a PR?

[nilsnolde]

probably worth to look at the isochrones (sorry I meant matrix, this is about isochrones 😅 ..) as well.

[mthh]

Sure, will do !