-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Deprecate unused parameters #16096
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
Deprecate unused parameters #16096
Conversation
lib/matplotlib/backend_bases.py
Outdated
@@ -2693,6 +2693,10 @@ def set_message(self, s): | |||
|
|||
def back(self, *args): | |||
"""Move back up the view lim stack.""" | |||
if args: |
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.
#16063 gives you @_delete_parameter(version, "args")
and @_delete_parameter(version, "kwargs")
.
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.
If that gets in first, I'll adapt, but wouldn't wait for this. BTW, I've approved #16063.
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.
You can also steal the implementation of _delete_parameter if you want.
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'll leave that to you 😄
bb7db6d
to
2a22dfc
Compare
@@ -53,7 +53,8 @@ class Tick(martist.Artist): | |||
The right/top tick label. | |||
|
|||
""" | |||
def __init__(self, axes, loc, label, | |||
@cbook._delete_parameter("3.3", "label") | |||
def __init__(self, axes, loc, label=None, |
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.
it's a bit strange because the Tick now takes a bunch of kwargs which basically get forwarded to the label Text objects (labelsize, labelcolor, labelrotation) but can't set the text itself? (IOW should we make the label kwarg actually work instead?) Alternatively should these labelfoos also get deprecated?
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 don't know. The label text is not really in the control of the tick but in the control of the Locator/Formatter. All usages in our codebase just passed an empty string. So making label
work seems not too desirable.
I think labelfoos are currently used for creating additional ticks with the right properties after set_tick_params()
was used. Would have to check for that.
Generally, I'd rather make the Tick
API smaller than larger because it's a bit performance critical; and we may want to migrate to somthing like a TickCollection at some point.
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 strongly agree with making it smaller rather than larger, but would rather get rid of all label related args together if possible. Won't block over that though.
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 look into that, but would rather do in a separate PR. I have the itch that everything will break down if I remove the labelfoos. And sorting that out in a small clean PR is simpler. Also it's easier to revert if we find out this was a bad idea.
2a22dfc
to
5ea4c1e
Compare
4729c7b
to
7f46eef
Compare
7f46eef
to
aeb8282
Compare
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.
postci
aeb8282
to
bb98d01
Compare
bb98d01
to
d8f815d
Compare
- matplotlib/matplotlib#16096 - matplotlib/matplotlib@SHA d8f815d36b204af86ea39734588aa5f771fbc45f
- matplotlib/matplotlib#16096 - matplotlib/matplotlib@SHA: d8f815d36b204af86ea39734588aa5f771fbc45f
PR Summary
Unused parameters to public functions.