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

Adding possibility to remove invisible lines and patches from relim #2417

Merged
merged 2 commits into from Sep 18, 2013

Conversation

fariza
Copy link
Member

@fariza fariza commented Sep 12, 2013

As discussed long time ago in the mailing list, and following the suggestion (copy/paste) from John Hunter http://sourceforge.net/mailarchive/forum.php?thread_name=4F5FB1F7.9050909%40hawaii.edu&forum_name=matplotlib-users

Added the possibility to exclude the invisible lines from relim.
The logic behind this is, that if we make a line invisible it is possible that we want to concentrate our attention to the visible ones, and this may require a relim.

I have created a pull-request before, but my inexperience with git, made that I completely screw the branch, so I have to redoit.
Sorry... my bad

@fariza
Copy link
Member Author

fariza commented Sep 14, 2013

The error seems unrelated to this modification.
This is my first PR, so I am learning along the way

@pelson
Copy link
Member

pelson commented Sep 16, 2013

The error seems unrelated to this modification.

Yep, I agree. The test failure is unrelated.

This is my first PR, so I am learning along the way

That's great. Welcome to the club 😄 - your effort is much appreciated!

@fariza
Copy link
Member Author

fariza commented Sep 16, 2013

Thanks.
One remaining question.... now what? Just wait or do I have to do something to get the PR accepted?

Thanks

@mdboom
Copy link
Member

mdboom commented Sep 16, 2013

Usually we leave this up for discussion for a while, in case anyone sees a downside that might be missed.

We have a list of requirements for a complete pull request that we try to follow (though in practice we sometimes miss things).

http://matplotlib.org/devel/coding_guide.html

I think for something this simple, you're mostly good on these counts, but it would be good to have a unit test for the new feature.

@fariza
Copy link
Member Author

fariza commented Sep 16, 2013

Added simple test, is this ok?

@pelson
Copy link
Member

pelson commented Sep 17, 2013

Added simple test, is this ok?

Lovely test. 👍

pelson added a commit that referenced this pull request Sep 18, 2013
Adding possibility to remove invisible lines and patches from relim.
@pelson pelson merged commit 750e2f4 into matplotlib:master Sep 18, 2013
@pelson
Copy link
Member

pelson commented Sep 18, 2013

Congratulations on getting your first merge to matplotlib. Welcome aboard 🚢

@fariza fariza deleted the remlim-with-invisible-option branch September 18, 2013 13:09
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