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

line interpolate point across MetricSpaces #1321

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

michaelkirk
Copy link
Member

@michaelkirk michaelkirk commented Mar 4, 2025

  • I agree to follow the project's code of conduct.
  • I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

LineInterpolatePoint is currently Euclidean only. As part of #1216, I wanted to make this functionality work across metric spaces.

// geos
line_interpolate_point(line, 0.8, true)
// georust::geo
Euclidean.interpolate_point_at_ratio_from_start(line, 0.8)

// also:
Haversine.interpolate_point_at_ratio_from_start(line, 0.8)
Geodesic.interpolate_point_at_ratio_from_start(line, 0.8)
Rhumb.interpolate_point_at_ratio_from_start(line, 0.8)

As a bonus, I also wanted to add some (IMO) long missing niceties to the api.

// geos communicates "from end" with a negative distance
line_interpolate_point(line, -0.8, true)
// georust::geo
Euclidean.interpolate_point_at_ratio_from_end(line, 0.8)
// geos communicates "distance" rather than "ratio" by passing in `false`
line_interpolate_point(line, 0.8, false)
// georust::geo
Euclidean.interpolate_point_at_distance_from_start(line, 0.8)
// geos
line_interpolate_point(line, -0.8, false)
// georust::geo
Euclidean.interpolate_point_at_distance_from_end(line, 0.8)

You can use all those methods across metric spaces:

Haversine.interpolate_point_at_ratio_from_end(line, 0.8)
Geodesic.interpolate_point_at_distance_from_start(line, 0.8)
Rhumb.interpolate_point_at_distance_from_end(line, 0.8)
// etc.

The names are admittedly long. It's my own preference to have verbose method names over geos-like "mode" parameters.

Bonus

While working on this, and filling out the deprecations, I found and fixed a bug in LineStringSegmentizeHaversine which was using the legacy LineInterpolatePoint, which was Euclidean only. It's easy to make this kind of mistake, the way we've historically not been very explicit about which metric space an algorithm is for.

@michaelkirk michaelkirk force-pushed the mkirk/line-interpolate-point branch from ba43f1a to af415da Compare March 4, 2025 04:40
@michaelkirk
Copy link
Member Author

This is a big PR, but it's overwhelmingly docs and tests, I swear!

@michaelkirk michaelkirk force-pushed the mkirk/line-interpolate-point branch 2 times, most recently from 6737a63 to 29eb15a Compare March 4, 2025 05:06
---- algorithm::linestring_segment::test::haversine_segment_length stdout ----
thread 'algorithm::linestring_segment::test::haversine_segment_length' panicked at geo/src/algorithm/linestring_segment.rs:333:13:
assert_relative_eq!(Haversine.length(&segment), expected_segment_length, epsilon = 1e-9)

    left  = 300003.0789340123
    right = 299799.8099830805
…tart, not (Euclidean) line_interpolate_point
@michaelkirk michaelkirk force-pushed the mkirk/line-interpolate-point branch 2 times, most recently from f4b888c to a18e16d Compare March 4, 2025 18:01
@michaelkirk michaelkirk force-pushed the mkirk/line-interpolate-point branch from a18e16d to d421459 Compare March 4, 2025 18:22
/// assert!(lines.next().is_none());
/// ```
pub fn rev_lines(&'_ self) -> impl ExactSizeIterator<Item = Line<T>> + '_ {
self.0.windows(2).rev().map(|w| {
Copy link
Member Author

@michaelkirk michaelkirk Mar 5, 2025

Choose a reason for hiding this comment

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

At first I assumed I wanted to do line_string.lines().rev() but that's not quite what I needed.

For example, given a line string with points a, b, c:

lines():
1. a->b
2. b->c
lines().rev():
1. b->c
2. a->b

But what I want is:

rev_lines():
1. c->b
2. b->a

@michaelkirk
Copy link
Member Author

I've just pushed up a bike-shedding commit.

There are two equivalent new APIs:

Euclidean.point_at_ratio_from_start(&line_string, 0.5)
line_string.point_at_ratio_from_start(&Euclidean, 0.5)

Previously, the deprecation pointed you to the first version. I changed that deprecation to now recommend the latter.

I don't think it matters much, but the second version is closer to the existing API, so I think it'll be slightly easier to migrate to.

segment.line_interpolate_point((length - remainder) / length)?;

let endpoint = $metric_space
.point_at_ratio_from_start(&segment, (length - remainder) / length);
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a bugfix in the case of Haversine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant