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

Implement Haversine distance #62

Closed
wants to merge 3 commits into from
Closed

Implement Haversine distance #62

wants to merge 3 commits into from

Conversation

tmcw
Copy link
Contributor

@tmcw tmcw commented Aug 21, 2016

No description provided.

@tmcw
Copy link
Contributor Author

tmcw commented Aug 21, 2016

Still learning! Anyone know whether there's a better way to avoid this longwinded series of T:from_i32 statements for an algorithm that works with both variables and constants?

@notriddle
Copy link

thread 'main' panicked at 'assertion failed: `(left == right)` (left: `10900.115611845817`, right: `10900.115612674515`)', <anon>:8

Something's going to have to be done about that.

@TeXitoi
Copy link
Contributor

TeXitoi commented Aug 30, 2016

Please remove the warnings.

@tmcw
Copy link
Contributor Author

tmcw commented Aug 30, 2016

Indeed, I'm very aware of the error, and will work on fixing it, nobody is expecting this PR to be merged with failing tests :)

Anyone know the answer to the T::from_i32 question, though?

@urschrei
Copy link
Member

I've had the same issue when using "actual numbers" within generic functions, and haven't been able to get around it, but I also haven't taken the time to ask anyone who works on/with num. It's probably worth asking on #rust, or users.rust-lang.org.

@urschrei
Copy link
Member

OK, so the consensus seems to be something like:

fn f<T: FromPrimitive>(i: i32) -> T { T::from_i32(i).unwrap() }

I'd assumed it would be more complicated than that.

@frewsxcv
Copy link
Member

Just rebased this off master. @tmcw It looks like there are failing tests. Do you know why they're failing? It'd be great if this could merge before #85, but no worries if you're short on time, just let me know :)

@Turbo87
Copy link
Member

Turbo87 commented Feb 14, 2017

@tmcw @frewsxcv this is most likely related to comparing floats using assert_eq(). we should probably use within_epsilon() for the assertions (see https://github.com/georust/rust-geo/blob/master/src/algorithm/distance.rs#L347)

@frewsxcv frewsxcv closed this in #90 Feb 18, 2017
@frewsxcv frewsxcv deleted the distance-two branch June 4, 2018 03:43
nyurik pushed a commit to nyurik/geo that referenced this pull request Mar 19, 2022
62: deserialize point r=frewsxcv a=michaelkirk

- [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/master/CODE_OF_CONDUCT.md).
- [n/a] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users.
  -  > Add helper function for deserializing from WKT format into geo_types::Geometry
    
      I think the existing unreleased note pretty much covers it 
---

Addresses one little bit of georust/wkt#61, though leaves unanswered the bigger questions, around how to handle `GEOMETRYCOLLETION(POINT EMPTY)` etc.

This might be controversial as it arguably muddies the water for addressing georust#61 more comprehensively. Please take a look at georust#61 for context.

Co-authored-by: Michael Kirk <michael.code@endoftheworl.de>
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.

6 participants