-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Revert change of parameter name in annotate() #12383
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
Revert change of parameter name in annotate() #12383
Conversation
|
ftr there's #7966 re: signature-overloaded functions, which can easily be adapted to handle deprecations in signature changes. |
anntzer
left a comment
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 think doing an additional deprecation cycle for this specific un-change is really not worth it.
6021a60 to
edaeca4
Compare
|
I think this is a bug fix for |
efiring
left a comment
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.
Looks fine; no need for anything more than this.
|
As a general comment, I think that sticking with "s" is the way to go. It stands for "string", contrasts adequately with "xy", avoids confusion or duplication with the |
PR Summary
Change the parameter name
annotate(text, ...)back toannotate(s, ...)(was introduced in #10830).Fixes #12325.
Not sure if it's ok to go back straight. IMO, it's rather uncommon to pass this param as kwarg. But technically, that's an API change again. If people have adapted for
s -> textin 3.0, they would have to adapt back again.Possible alternative
Generally, I'd rather have descriptive argument names such as
textortitleinstead ofs. However, I'm learning to be more careful with API changes and these things would have to go through a deprecation mechanism with a transition phase of allowing both. If there is a general approval to migrate parameter names in such a way, I can apply the mechanism here and just addsback in as a fallback (including changing the originalAnnotationclass as well.Note: Thinking about it, a transition mechanism is technically possible, however a bit complicated. We would have to support:
annotate('text', xy)annotate(s='text', xy=xy)annotate(text='text', xy=xy)which would require