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

Map coords takes a coordinate #837

Merged
merged 2 commits into from
Jun 3, 2022
Merged

Map coords takes a coordinate #837

merged 2 commits into from
Jun 3, 2022

Conversation

michaelkirk
Copy link
Member

  • 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.

FIXES #804

Note there will be a little conflict with #836 - whoever merges second will have to make a small update.

Sorry, something went wrong.

#[deprecated(
since = "0.21.0",
note = "use `MapCoords::try_map_coords` which takes a `Coordinate` instead of an (x,y) tuple"
)]
pub trait TryMapCoords<T, NT, E> {
Copy link
Member Author

@michaelkirk michaelkirk Jun 2, 2022

Choose a reason for hiding this comment

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

Note, for the deprecated methods, I left the signatures as (T, T) tuples, figuring they'll go away soon enough and this way we can better have the compiler tell the users what to do.

Copy link
Member

@frewsxcv frewsxcv left a comment

Choose a reason for hiding this comment

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

lgtm, though you'll need to rebase/merge after my other pr merges

/// use approx::assert_relative_eq;
///
/// let p1 = Point::new(10., 20.);
/// let p2 = p1.map_coords(|(x, y)| (x + 1000., y * 2.));
/// let p2 = p1.map_coords(|Coordinate { x, y }| Coordinate { x: x + 1000., y: y * 2. });
///
/// assert_relative_eq!(p2, Point::new(1010., 40.), epsilon = 1e-6);
/// ```
///
/// You can convert the coordinate type this way as well
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated, but we should probs advertise Convert here now?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've rebased, and pushed up a new commit which references Convert for the case of simply changing numeric type , and then updated this example to solve a more complex problem involving both type conversion and a transformation for which Convert would be insufficient.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@michaelkirk
Copy link
Member Author

bors r=frewsxcv

@bors
Copy link
Contributor

bors bot commented Jun 3, 2022

Build succeeded:

@bors bors bot merged commit 959ed04 into main Jun 3, 2022
@bors bors bot deleted the mkirk/map-coords branch June 3, 2022 06:24
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.

Replace MapCoords tuple parameter in function argument with a Coodinate parameter
2 participants