Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Deprecate positional use of most arguments of plotting functions#27786
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| Many plotting functions will restrict positional arguments to the first few parameters | ||
| in the future. All further configuration parameters will have to be passed as keyword |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is there any way to have some measure of consistency on which parameters are positional only (like for example the data parameters?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The central question is:Is code using the positional parameters understandable without knowing the signature? Mostly this results in positional data parameters. But with some exceptions:
fmtis allowed positionally - inspired byplot- I’ve still allowed
binsin histograms because fromhist(data, 20)it's conceivable that the inter is bins(debatable, but err on the permissive side) - With
hlinesI've gone to the extreme and still allowed color and linestyles because if somebody has writtenhlines(y, xmin, xmax, 'red', 'dashed')that's quite clear and I don't want to break their code.
There seems to be an order dependence in matplotlib/tools/boilerplate.py Lines 160 to 165 in1defb57
For instance, >>>mpl.__version__'3.9.0.dev1136+g3edfce3b0e'>>>mpl._api.deprecation.DECORATORS.get(mpl.axes.Axes.violinplot)>>>mpl._api.deprecation.DECORATORS.get(mpl.axes.Axes.violinplot.__wrapped__)functools.partial(<functionmake_keyword_onlyat0x7f641386bce0>,'3.9','vert')>>>mpl._api.deprecation.DECORATORS.get(mpl.axes.Axes.violin)functools.partial(<functionmake_keyword_onlyat0x7f641386bce0>,'3.9','vert') If you reach into
Thus I think the easiest corrective action to make pyplot not prematurely implement kwonly args (without warning) is to move the deprecations to be the outermost decorators... That or the boilerplate code needs to grow to search the entire At first I thought it was some of the code I wrote in boilerplate to add type hints in, but I confirmed that I do not change the param types from the |
Uh oh!
There was an error while loading.Please reload this page.
7e10766 to87b8b2eCompareIn discussion with@tacaswell and@QuLogic we decided to push this to early 3.10 rather than late 3.9. |
This increases maintainability for developers and disallowsbad practice of passing lots of positional arguments for users.If in doubt, I've erred on the side of allowing as much positionalargumentsas possible as long as the intent of a call is still readable.Note: This was originally motivated by bxp() and boxplot() having manyoverlapping parameters but differently ordered.While at it, I think it's better to rollout the change toall plotting functions and communicate that in one gorather than having lots of individual change notices overtime. Also, the freedom to reorder parameters only setsin after the deprecation. So let's start this now.
... to the error message, that gets thrown if it is not.
| @@ -4636,6 +4644,7 @@ def invalid_shape_exception(csize, xsize): | |||
| colors = None # use cmap, norm after collection is created | |||
| return c, colors, edgecolors | |||
| @_api.make_keyword_only("3.9", "marker") | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I can see an arguament for pushing this one deeper
ax.scatter(x,y,s,c,'o')
scans reasonably well, but I suspect is rare.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We should be open to feedback on these if they turn out to be more disruptive than we anticipate.
The appveyor failure is issues with gobject importing so I'm going to merge over that. I'm going to merge this (as we have branched for 3.9) to get it in as early as possible and to make the odds of finding major disruptions down stream as high as possible. I'm torn if the deprecation note on this is exuberant enough for the scale of the change. |
I believe most affected users will learn from the runtime warning anyway. But feel free to change the note. |
The warnings still say 3.9 👀 Should we correct them this far down the path? |
It doesn't matter too much. But we should watch out that we don't shorten expiration because we noted a version number too low. So likely the simplest thing is updating. |
This increases maintainability for developers and disallows bad practice of passing lots of positional arguments for users. If in doubt, I've erred on the side of allowing as much positional arguments
as possible as long as the intent of a call is still readable.
Note: This was originally motivated by bxp() and boxplot() having many overlapping parameters but differently ordered.
While at it, I think it's better to rollout the change to all plotting functions and communicate that in one go rather than having lots of individual change notices over time. Also, the freedom to reorder parameters only sets in after the deprecation. So let's start this now.