Skip to content

Adding snapping distances to route results#136

Open
e-kotov wants to merge 9 commits intoriatelab:masterfrom
e-kotov:snap-dist
Open

Adding snapping distances to route results#136
e-kotov wants to merge 9 commits intoriatelab:masterfrom
e-kotov:snap-dist

Conversation

@e-kotov
Copy link

@e-kotov e-kotov commented Feb 9, 2026

OSRM snaps input coordinates to the nearest road to calculate a route. Previously, the package didn't show how far the points were moved unless you made extra calls to osrmNearest and/or pre-snapped the points before routing.

I updated osrmRoute, osrmTable, and osrmTrip to include these snapping distances directly in the output.

  • osrmRoute now includes src_snapping_distance and dst_snapping_distance columns (in km). For routes with multiple points, the full metadata for every point is stored in a snapping attribute, which can be accessed with attr(res, "snapping").

  • osrmTable now supports measure = "total_distance". This returns a matrix where the snapping distances of the source and destination are already added to the network distance.

  • osrmTrip now includes a waypoints element in the returned list, showing the snapping_distance for every stop in the trip.

These changes make it easier to see the total distance without making additional API calls as all of the information is already in the response anyway.

Expose OSRM waypoint snapping distances in route results: add `src_snapping_distance` and `dst_snapping_distance` fields to the returned sf object, attach full snapped waypoints as a `snapping` attribute, and include snapping distances in source/destination coordinate formatting. Update documentation and tests accordingly.
…e documentation

Convert the raw distance in the snapping waypoints from meters to kilometers, rename it to `snapping_distance`, and update documentation and tests accordingly.
Copilot AI review requested due to automatic review settings February 9, 2026 22:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR exposes OSRM “snapping” (distance from input point to the snapped network point) directly in osrmRoute, osrmTable, and osrmTrip results so users don’t need separate osrmNearest calls to quantify point displacement.

Changes:

  • osrmRoute: adds src_snapping_distance / dst_snapping_distance columns and a snapping attribute containing waypoint snap metadata.
  • osrmTable: adds measure = "total_distance" and includes snapping distances in sources/destinations.
  • osrmTrip: returns snapped waypoints (sf POINT) with snapping_distance.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
man/osrmTrip.Rd Documents new waypoints element for trip results.
man/osrmTable.Rd Updates docs for new snapping-inclusive distance measure/output (currently inconsistent with implementation).
man/osrmRoute.Rd Documents new snapping distance columns and snapping attribute.
R/osrmTrip.R Constructs waypoint snapping distances and returns them as sf points per trip.
R/osrmTable.R Implements measure="total_distance" and computes total_distances including snapping.
R/osrmRoute.R Adds snapping distance columns and stores full waypoint snap info as an attribute.
R/utils.R Adds snapping_distance fields to coord_format() outputs.
inst/tinytest/test_osrmTrip.R Adds assertions for returned waypoints and snapping_distance.
inst/tinytest/test_osrmTable.R Adds coverage for measure="total_distance" and expected arithmetic.
inst/tinytest/test_osrmRoute.R Updates expectations for new columns + snapping attribute.
inst/tinytest/coord_format_out.rds Updates test fixture for coordinate formatting output.
Comments suppressed due to low confidence (1)

R/osrmRoute.R:225

  • rosf is reprojected back to the input CRS when oprj is set, but the newly added snapping attribute remains in the OSRM response coordinate space (EPSG:4326). This makes the route geometry CRS and waypoint coordinates inconsistent for sf inputs in other CRSs. Consider transforming the snapped waypoint locations to oprj as well (e.g., store them as an sf POINT object like osrmTrip does), or explicitly document that the snapping attribute is always WGS84.
  snapping <- res$waypoints
  snapping$snapping_distance <- snapping$distance / 1000
  snapping$distance <- NULL
  attr(rosf, "snapping") <- snapping
  # prj
  if (!is.na(oprj)) {
    rosf <- st_transform(rosf, oprj)
  }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

e-kotov and others added 3 commits February 10, 2026 00:02
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@rCarto
Copy link
Member

rCarto commented Feb 11, 2026

Hello @e-kotov ,
Thank you for the PR, that's a good idea to add the snapping distance where it's relevant.
I have a few comments before accepting the PR.

For osmRoute()
I'm not in favor of adding a "snapping" attribute table to the result. I would prefer adding a "snapping_distance" argument (default to FALSE) to osrmRoute(). If TRUE the result is a list : first element is the route, second element is the snapping table.
The snapping table should only contain ids and the snapping distances.
The snapping table could be a sf POINT layer (with same CRS as input)? What do you think?

For osrmTable()
The snapping distance unit should be indicated in the help page.
The total_distances table should be placed before sources and destinations tables in the result

For osrmTrip()
The waypoints table should only contain loc ids and the snapping distances.

@e-kotov
Copy link
Author

e-kotov commented Feb 11, 2026

For osmRoute()
I'm not in favor of adding a "snapping" attribute table to the result. I would prefer adding a "snapping_distance" argument (default to FALSE) to osrmRoute(). If TRUE the result is a list : first element is the route, second element is the snapping table.
The snapping table should only contain ids and the snapping distances.
The snapping table could be a sf POINT layer (with same CRS as input)? What do you think?

Good call. I agree attribute is very awkward. I just was not sure if you would want to alter the return value of the function to change or not. Attribute allowed to do it surgically with not significant change, but since you suggested it, I will definitely refactor it to be an optional list.

For osrmTable()
The snapping distance unit should be indicated in the help page.

Sure, will do.

The total_distances table should be placed before sources and destinations tables in the result

But what if someone is using osrm and relying on the existing order of the items in the list? I is bad practice to rely on order, but I would not rule this out.

For osrmTrip()
The waypoints table should only contain loc ids and the snapping distances.

Ok, will do.

@rCarto
Copy link
Member

rCarto commented Feb 11, 2026

But what if someone is using osrm and relying on the existing order of the items in the list? I is bad practice to rely on order, but I would not rule this out.

Ok you're right, let's keep it where it is then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants