Skip to content

Improve deprecation documentation. Closes #7643 #7646

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 3 commits into from
Dec 20, 2016

Conversation

ngoldbaum
Copy link
Contributor

This improves the deprecation warnings printed by axes.hold and plt.hold and adds a mention of the deprecation of the spectral colormap to the API changes document.

@NelleV NelleV changed the title Improve deprecation documentation. Closes #7643 [MRG+1] Improve deprecation documentation. Closes #7643 Dec 20, 2016
@NelleV NelleV added this to the 2.0 (style change major release) milestone Dec 20, 2016
@NelleV
Copy link
Member

NelleV commented Dec 20, 2016

LGTM. Thanks for the patch!

@efiring
Copy link
Member

efiring commented Dec 20, 2016

My reading of the deprecation decorator code is that if you want to use your own message, you would normally include some format strings. At present I think your PR is discarding the version information.

Actually, it looks to me like the underlying _generate_deprecation_message is broken by indentation errors, at the very least... It is not doing the substitutions it should if a message is given.

@NelleV
Copy link
Member

NelleV commented Dec 20, 2016

@efiring There are two deprecation messages. Which one are you talking about?

@efiring
Copy link
Member

efiring commented Dec 20, 2016

Both of them. Same problem. The first one could be helpful if it included the necessary format strings, and if the _generate_deprecation_message bug were fixed. The second one is pointless, providing less information than what it replaces.

@NelleV
Copy link
Member

NelleV commented Dec 20, 2016

The deprecation messages don't strike me as being less informative. IMO, it is quite the opposite.

@efiring
Copy link
Member

efiring commented Dec 20, 2016

@NelleV you are correct; given that the decorator replaces the default message with the one supplied as an argument, ignoring all other arguments, this PR does provide more information. I had an incorrect expectation of what the decorator would do in this case. (Nevertheless, it is broken in that it can't presently be used as intended, taking advantage of its arguments via format strings in the message).

@ngoldbaum
Copy link
Contributor Author

Yes, I noticed last night that the format string magic wasn't working.

@tacaswell
Copy link
Member

appveyor failure looks transient.

@tacaswell tacaswell merged commit c34ddc4 into matplotlib:master Dec 20, 2016
tacaswell added a commit that referenced this pull request Dec 20, 2016
DOC: Improve deprecation documentation (hold and spectral)

Closes #7643
@tacaswell
Copy link
Member

Thanks @ngoldbaum !

backported to v2.x as 4f0d2ee

@efiring
Copy link
Member

efiring commented Dec 20, 2016

See also #7651.

@QuLogic QuLogic changed the title [MRG+1] Improve deprecation documentation. Closes #7643 Improve deprecation documentation. Closes #7643 Dec 26, 2016
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