Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add CoordsIter trait for iterating over coordinates in geometries. #164

Merged
merged 26 commits into from
Nov 17, 2020

Conversation

frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Sep 19, 2017

A new trait to iterate over all coordinates in a geometry. In the future, we should add a new method to this trait called external_coords_iter which iterates over the external coordinates of a geometry, excluding coordinates of interior holes. Then we can make algorithms like ConvexHull have a generic implementation where G: CoordsIter

@frewsxcv frewsxcv force-pushed the frewsxcv-coords-iter branch from ba17ca3 to ddae160 Compare September 19, 2017 03:29
@frewsxcv
Copy link
Member Author

working on refactoring some of the rust-geo traits right now. this is similar to MapCoords but unlike MapCoords the methods on this trait don't result in any new memory allocations.

@frewsxcv frewsxcv force-pushed the frewsxcv-coords-iter branch from ddae160 to bf85e8d Compare September 19, 2017 12:40
@amandasaurus
Copy link
Member

I have a map_coords_inplace branch lying around at home that I should make a PR for. It allows you to change things without memory allocations But maybe this would suffice.

@frewsxcv
Copy link
Member Author

It allows you to change things without memory allocations But maybe this would suffice.

yep, the main reason i opened this is so that i can modify Coordinate values without allocations:

https://github.com/georust/rust-geo/compare/frewsxcv-rotate-refactgor?expand=1#diff-64481126a8d2c515ad92dc33cc71a856R18

@amandasaurus
Copy link
Member

:) I also wanted to modify objects without allocations.

I should clean up my code and start opening PRs

@frewsxcv
Copy link
Member Author

@rory don't want to pressure you into reviewing, but did you happen to already look at the code here? if so, any thoughts?

@amandasaurus
Copy link
Member

I only skimmed it earlier. I might be able to look at it properly within the next few days

MultiLineString};

pub trait CoordsIter<T: Float> {
fn coords_iter<'a>(&'a self) -> Box<Iterator<Item = &'a Coordinate<T>> + 'a>;
Copy link
Member Author

Choose a reason for hiding this comment

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

just like the other boxed iterators that get returned in this crate, we can remove the Box once return impls stabilize

@amandasaurus amandasaurus mentioned this pull request Sep 23, 2017
bors bot added a commit that referenced this pull request Mar 1, 2018
170: Map coords inplace r=frewsxcv a=rory

To compliment the `.map_coords()` feature, I've added a `.map_coords_inplace` which will apply a provided function to all coordinatines in place, i.e. not doing any allocations.

Some of this has a lot of overlap with #164.

I've used that to add a `.translate_inplace()` feature.
@frewsxcv frewsxcv closed this Sep 22, 2020
@frewsxcv frewsxcv reopened this Nov 14, 2020
@frewsxcv
Copy link
Member Author

just finished this! i'll try to find some time in the next few days to add tests. most of the time i spent on this was trying to remove Box<Iterator>, which is why we have to specify the specific types of the iterator for each implementation

@frewsxcv
Copy link
Member Author

also will add docs to the trait. just realized they're missing

Copy link
Member

@michaelkirk michaelkirk left a comment

Choose a reason for hiding this comment

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

nice!

@frewsxcv
Copy link
Member Author

bors r=michaelkirk

@bors
Copy link
Contributor

bors bot commented Nov 17, 2020

Build succeeded:

@bors bors bot merged commit 989f962 into master Nov 17, 2020
@frewsxcv frewsxcv deleted the frewsxcv-coords-iter branch November 18, 2020 14:42
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants