Added the definition of Deprecation and made Deprecation Process clearer · Pull Request #21626 · matplotlib/matplotlib · GitHub
Skip to content

Added the definition of Deprecation and made Deprecation Process clearer - #21626

Merged
timhoffm merged 6 commits into
mainfrom
unknown repository
Nov 16, 2021
Merged

timhoffm merged 6 commits into
mainfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Nov 13, 2021

Copy link
Copy Markdown

#21393 @story645 @anntzer @QuLogic @jklymak

I suggest the following changes to make things a bit clearer.

@timhoffm

Copy link
Copy Markdown
Member

@timhoffm timhoffm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@sveneschlbeck, sorry, I started with single comments, but basically ended up rewriting the whole section 😮

Comment thread doc/devel/contributing.rst Outdated
Comment thread doc/devel/contributing.rst Outdated
Sven Eschlbeck and others added 2 commits November 15, 2021 09:02
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
@ghost

ghost commented Nov 15, 2021

Copy link
Copy Markdown
Author

@timhoffm No worries, I am learning a lot from you along the process, so no problem. All suggestions were right and good.

- The deprecated API should, to the maximum extent possible, remain fully
functional during the deprecation period. In cases where this is not
possible, the deprecation must never make a given piece of code do something
different than it was before; at least an exception should be raised.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this point ("must never make a given piece of code...") is important and must be kept (possibly reworded).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@jklymak @timhoffm Do you feel the same way regarding this statement? I think @timhoffm consciously changed that part to make it clear that the old API must (not should) remain functional. Therefore, any ",but..." clause is irrelevant. BTW, this is not my definite opinion, I am just paraphrasing what I think @timhoffm said and I can follow this logic. If it MUST stay functional than we don't need to worry about the "what if it does'nt remain functional" case

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm fine with the more stringent statement to keep things simple and as a guide to contributors.

Very occasionally, this list of rules ends up being bypassed. Since this basically has to be done by the lead devs, listing the caveats and how to mitigate them seems unnecessary in a document like this. However, if I were writing the first sentence of the second paragraph I would say

API changes in Matplotlib have to be performed following the deprecation process below, except in very rare circumstances as deemed necessary by the development team.

That makes it clear we can't always live up to the ideals of this process...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm OK with @jklymak's take on this.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Alright, sounds like a good compromise

Comment thread doc/devel/contributing.rst Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't introduce only one subsection of a section. If you want subsections, suggest "Rules" "Introducing..." and "Expiring..." I think it is obvious that these are the "deprecation process"

@ghost ghost Nov 15, 2021

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Alright, changed the subsection titles and got rid of the obvious (too generic) title "Deprecation process"

Introduced more specific subsection titles since it is obvious that all refer to Deprecations
Adding sentence saying that rare cases might need lead dev attention
@timhoffm timhoffm added this to the v3.5.1 milestone Nov 16, 2021
@timhoffm
timhoffm merged commit 49609c3 into matplotlib:main Nov 16, 2021
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Nov 16, 2021
@ghost
ghost deleted the api_doc_change branch November 16, 2021 16:35
timhoffm added a commit that referenced this pull request Nov 16, 2021
…626-on-v3.5.x

Backport PR #21626 on branch v3.5.x (Added the definition of Deprecation and made Deprecation Process clearer)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants