-
-
Notifications
You must be signed in to change notification settings - Fork 8k
Correctly handle Axes subclasses that override cla #23735
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
Conversation
I'm pretty sure we did not intend to remove or deprecate
Axes.cla
given its wide use, though I'm not sure if we wanted to discourage it (by undocumenting or just putting that in the docstring.)
We cannot deprecate Axes.cla
as an interface because it's too widely used. But I think we should discourage it (docstring), because clear()
is much more clear 😜, and in general we should give guidance which way to use if we really need to have to ways of doing the same thing.
I tend to say that we also should nudge downstream libraries to override clear()
instead of cla()
. - i.e. put a pending deprecation in the init_sublcass "If you want to override clearing Axes, override "Axes.clear" instead of "Axes.cla". "Axes.cla" will only be an alias delegating to clear
in the future, and thus standard clear operations will not call an overriden cla
in the future."
This fixes, e.g., Cartopy, but probably most other third-party packages that will subclass `Axes`.
b0106e9
to
74b2c2d
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.
This works as expected with Cartopy's various versions and emitting the PendingDeprecation. One thing that arises is the redefinition of _clear()
in the subclasses. I wonder if that name is a bit special and also potentially used by other downstream libraries already and this would also get them into trouble?
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
@QuLogic can merge this is happy with the last 2 commits.
...735-on-v3.6.x Backport PR #23735 on branch v3.6.x (Correctly handle Axes subclasses that override cla)
PR Summary
In #22802,
Axes.clear
was made the primary interface overAxes.cla
. However, while this can be enforced internally, that can't be said for third-party subclasses. I'm pretty sure we did not intend to remove or deprecateAxes.cla
given its wide use, though I'm not sure if we wanted to discourage it (by undocumenting or just putting that in the docstring.)This fixes, e.g., Cartopy, but probably most other third-party packages that will subclass
Axes
.PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).