Skip to content

Deprecate original NavigationToolbar #1388

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

Merged
merged 2 commits into from
Oct 15, 2012

Conversation

efiring
Copy link
Member

@efiring efiring commented Oct 13, 2012

There seemed to be agreement on the mailing list that we could deprecate the original NavigationToolbar in the 1.2 release. In this PR I have done the most minimal version of deprecation, that is, putting a warning in the rcParams validator.

@pelson
Copy link
Member

pelson commented Oct 14, 2012

In this PR I have done the most minimal version of deprecation, that is, putting a warning in the rcParams validator.

The alternative is deprecating each of the backend's NavigationToolbar classes (there is no superclass to do it in just one place). Based on this information, I agree with your approach of deprecating it for the majority of users (those who don't use pyplot to setup their figures won't necessarily get this message).

Is there an appropriate place to document this change? Perhaps the api_changes document?

The implementation looks good to me. +1

@dmcdougall
Copy link
Member

I also agree with the implementation here.

@mdboom
Copy link
Member

mdboom commented Oct 15, 2012

This looks good to me. Deprecating the classes as @pelson suggests would also be fine, but I think the vast majority of users are probably choosing it through the rcParam, which this PR currently addresses.

efiring added a commit that referenced this pull request Oct 15, 2012
Deprecate original NavigationToolbar
@efiring efiring merged commit 58cc484 into matplotlib:v1.2.x Oct 15, 2012
@efiring efiring deleted the deprecate_Toolbar branch May 29, 2013 02:04
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.

4 participants