In contributing.rst, encourage kwonly args and minimizing public APIs. #14545
Conversation
... so that we can just point PR authors to it instead of explaining again and again why.
| to add as little public API as possible. In particular, make sure that helper | ||
| methods and internal attributes are private (i.e., start with an underscore). | ||
| Remember that any non-private method can be directly called by the end user, | ||
| and any non-private attribute can be directly set! |
There was a problem hiding this comment.
I'd probably leave out this sentence. It is extraneous and doesn't really explain why this is a "bad thing" (it can also confuse some contributors because they'll notice that they can do exactly the same thing with private methods and attributes).
There was a problem hiding this comment.
I'm not particularly in love with the wording here, but I do want to stress that point extremely strongly: if an attribute is public, the implementation should be robust against the end user modifying it; otherwise, make it private, or hide it behind a getter or a property.
If you have a better wording I'm all ears :)
There was a problem hiding this comment.
Or add a "And therefore once we have released that API the behavior, arguments, and names can not be changed without going through a multi-version deprecation process." ?
There was a problem hiding this comment.
Full agreement that we should document this better. (n.b. I'm planning to give a talk on API evolution at PyCon.DE).
You've said, you welcome rewording, so I wrote my own two paragraphs: 😄
API Changes
Changes to the public API must follow a standard deprecation procedure to prevent unexpected breaking of code that uses Matplotlib.
- Deprecations must be announced via an entry in
doc/api/next_api_changes. - Deprecations are targeted at the next point-release (i.e. 3.x.0).
- The respective API must still be fully functional during the deprecation period. If possible, usage of that API should emit a
MatplotlibDeprecationWarning(seecbook.warn_deprecated(), the@cbook.deprecateddecorator and other helper functions inmatplotlib.cbook.deprecationthat ease the deprecation of various aspects of an API). - Deprecated API may be removed two point-releases after they were deprecated.
Adding new API
Every new function, parameter and attribute that are not explicitly marked as private (i.e., start with an underscore) become part of Matplotlib's public API. As discussed above, changing the existing API is cumbersome. Therefore, take particular care when adding new API:
-
Mark helper functions and internal attributes as private by prefixing them with an underscore.
-
Carefully think about good names for your functions and variables.
-
Try to adopt patterns and naming conventions from existing parts of the Matplotlib API.
-
Consider making as many arguments keyword-only as possible.
.. seealso::
API Evolution the Right Way -- Add Parameters Compatibly__
__ https://emptysqua.re/blog/api-evolution-the-right-way/#adding-parameters
|
It's great :p Do you want to PR it separately, or should I just copy-paste that? |
|
Although I do want to comment on
I think my view is closer to (with pretty bad wording)
(Basically, if something changes and the user gets an exception, the user knows that, well, something needs to be fixed, or at least they'll pin to the previous release. If the behavior of the code changes, on the other hand, the user may well generate incorrect plots without ever realizing it.) See e.g. #14544... |
|
Feel free to use my text and adapt as proposed. It's getting late here. If nothing has been done tomorrow, I'll pick it up again. 😄 |
|
Replaced by #14555. |
... so that we can just point PR authors to it instead of explaining
again and again why.
Thought of this while looking at #14512.
Rewordings welcome.
PR Summary
PR Checklist