Skip to content
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

Fix draggable annotations on subfigures. #27343

Merged
merged 1 commit into from Dec 5, 2023
Merged

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Nov 17, 2023

  • Picking on subfigures was broken because ax = getattr(a, 'axes', None) would return the SubFigure.axes list when a is a subfigure, so the axes identity test would just be completely wrong. Fix that by special-casing subfigures. (Ideally I would entirely remove the axes equality test because that also breaks in the case of overlapping axes, but that's a much bigger change.)
  • ref_artist.figure._canvas_callbacks would fail when the ref_artist is on a subfigure because subfigures don't have a _canvas_callbacks attribute; instead use ref_artist.figure.canvas.callbacks where accessing the canvas attribute correctly goes back to the toplevel figure first.

See test for an example (which would previously fail with an AttributeError).

PR summary

PR checklist

- Picking on subfigures was broken because `ax = getattr(a, 'axes', None)`
  would return the SubFigure.axes *list* when `a` is a subfigure, so the
  axes identity test would just be completely wrong.  Fix that by
  special-casing subfigures.  (Ideally I would entirely remove the axes
  equality test because that also breaks in the case of overlapping axes,
  but that's a much bigger change.)
- `ref_artist.figure._canvas_callbacks` would fail when the ref_artist
  is on a subfigure because subfigures don't have a _canvas_callbacks
  attribute; instead use `ref_artist.figure.canvas.callbacks` where
  accessing the canvas attribute correctly goes back to the toplevel
  figure first.

See test for an example (which would previously fail with an
AttributeError).
@QuLogic
Copy link
Member

QuLogic commented Nov 18, 2023

There's a existing issue #25860 for the first part, and a PR #26916, though I've not compared it with this one.

@anntzer
Copy link
Contributor Author

anntzer commented Nov 18, 2023

From a quick look I think I still prefer my approach (#26916 compresses two recursion levels of the artist tree walking into a single one when the first level would incorrectly cause an entire subtree (associated with a SubFigure) to be skipped, whereas this PR explicitly disables subtree-skipping with SubFigures).

@anntzer anntzer added this to the v3.9.0 milestone Nov 24, 2023
@ksunden ksunden merged commit 461f0b0 into matplotlib:main Dec 5, 2023
43 checks passed
@anntzer anntzer deleted the sfad branch December 5, 2023 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants