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

RF - replace _filter_peaks with unique_vertices #34

Merged
merged 2 commits into from Jul 6, 2012

Conversation

MrBago
Copy link
Contributor

@MrBago MrBago commented Jul 4, 2012

This unique_vertices will be used by the HemiSphere and for fiber tracking.

@MrBago
Copy link
Contributor Author

MrBago commented Jul 4, 2012

Maybe just filter_vertices? Any other ideas for the name of a function which takes a set of vertices and returns a subset of those that are separated by at least some angle theta?

@Garyfallidis
Copy link
Contributor

filter_vertices could be using many different filters. The filter you are using is angular distance. So, the complete name and awesome name is angular_distance_filter !! :-)

@MrBago
Copy link
Contributor Author

MrBago commented Jul 5, 2012

How about remove_similar_vertices? Any takers?

@Garyfallidis
Copy link
Contributor

Yeah sure. Make the change and then I will happily do the merge.

Garyfallidis added a commit that referenced this pull request Jul 6, 2012
RF - replace _filter_peaks with unique_vertices
@Garyfallidis Garyfallidis merged commit ae7b20e into dipy:master Jul 6, 2012
@arokem
Copy link
Contributor

arokem commented Jul 6, 2012

@Garyfallidis This breaks the test suite (with several import errors). Please don't merge things in w/o making sure that the tests don't break. I think Stefan is working on fixing this right now. I will update you on this.

@Garyfallidis
Copy link
Contributor

Cool thx.

@Garyfallidis
Copy link
Contributor

I would love to hear what you guys think is the best merging strategy.

@arokem
Copy link
Contributor

arokem commented Jul 6, 2012

First try rebasing on top of the current master. Resolve any conflicts that occur when you do that. Then run the test suite. If the tests don't pass, go back to the pull request and tell everyone about it. Sort of like what I did here: #33

Iterate until all the tests pass and everyone is happy. Then, merge the feature branch into master (never the other way around!) and push master back to nipy/dipy

@MrBago
Copy link
Contributor Author

MrBago commented Jul 6, 2012

Generally I agree with you Ariel, but in this case we're rewriting several core modules so I've been trying to get them pulled one at a time so I can get feedback. I've found that very large pull requests do not get as much feedback. I'm working on updating the tests as we speak.

matthew-brett added a commit to matthew-brett/dipy that referenced this pull request Aug 4, 2014
This reverts commit e63c178, reversing
changes made to c273fe5.

We decided to back off these commits until we had checked the code
coverage of the Cython modules, or timed out doing that.

Discussion here:

dipy#34
skoudoro pushed a commit to fury-gl/fury that referenced this pull request Sep 20, 2018
This reverts commit e63c178c4dadeb635936fcf03daa84f9bf8f63f0, reversing
changes made to c273fe50bd389184ae069fd5a70187e38f699d4d.

We decided to back off these commits until we had checked the code
coverage of the Cython modules, or timed out doing that.

Discussion here:

dipy/dipy#34
skoudoro pushed a commit that referenced this pull request Oct 17, 2019
[fix] update test for import error
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.

None yet

3 participants