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

[Bug]: AnnotationBbox does not return correct window_extent before first draw #24453

Closed
tomicapretto opened this issue Nov 14, 2022 · 14 comments · Fixed by #26184
Closed

[Bug]: AnnotationBbox does not return correct window_extent before first draw #24453

tomicapretto opened this issue Nov 14, 2022 · 14 comments · Fixed by #26184
Labels
status: confirmed bug topic: geometry manager LayoutEngine, Constrained layout, Tight layout
Milestone

Comments

@tomicapretto
Copy link

Bug summary

I’m trying to use a constrained layout in a visualization that contains an artist that is an instance of AnnotationBbox, and matplotlib raises a warning saying constrained layout is not applied. The visual effect is not evident in this simple example, but it becomes very clear once we have multiple panels.

Code for reproduction

import matplotlib
import matplotlib.pyplot as plt
from matplotlib.offsetbox import AnnotationBbox, TextArea

fig, ax = plt.subplots(layout="constrained")

ab = AnnotationBbox(
    TextArea("Some text", textprops={"size": 42}),
    (0.5, 0.5),
    xycoords="axes fraction",
    box_alignment=(0.5, 0.5),
    pad=0
)

ax.add_artist(ab)
fig.set_facecolor("w")
fig.savefig("annotation_box.png", dpi=300)

Actual outcome

UserWarning: constrained_layout not applied because axes sizes collapsed to zero. Try making figure larger or axes decorations smaller.

Expected outcome

No warning should appear

Additional information

The following works without any warning

fig, ax = plt.subplots(layout="constrained")
ax.text(0.5, 0.5, "Some text", size=42, ha="center")
fig.set_facecolor("w")
fig.savefig("ax_text.png", dpi=300)

The problem with the constrained layout is more evident if I have two or more panels.
One way of fixing it (i.e. getting rid of the warning and bad functionality) is to do ab.set_in_layout(False) before doing ax.add_artist(ab).

This was first posted on Discourse https://discourse.matplotlib.org/t/constrained-layout-does-not-work-well-with-annotationbbox/23301

Operating system

Ubuntu 22

Matplotlib Version

3.6.2

Matplotlib Backend

No response

Python version

No response

Jupyter version

No response

Installation

pip

@jklymak
Copy link
Member

jklymak commented Nov 14, 2022

Try ab.set_in_layout(False) if you don't want an artist accounted for in constrained_layout.

Probably an easy todo here is to be more explicit about set_in_layout in the constrained layout tutorial. It is mentioned, but only in the context of a legend.

Edit:

sorry I see that you know about `set_in_layout(False). What bug are you reporting then?

@jklymak jklymak added Documentation Community support Users in need of help. labels Nov 14, 2022
@tacaswell
Copy link
Member

Why does an annotation box in an Axes who's content is also fully inside the Axes cause constrained layout to grow confused?

@jklymak
Copy link
Member

jklymak commented Nov 14, 2022

Not sure. I assume AnnotationBox doesn't know it's bounding box until after a draw? If you force a draw everything is fine. Btw doubt this is unique to constrained_layout.

@jklymak jklymak removed the Community support Users in need of help. label Nov 14, 2022
@jklymak
Copy link
Member

jklymak commented Nov 14, 2022

If you do ab.get_tightbbox() before a draw you get Bbox(x0=0.0, y0=-6.0, x1=142.125, y1=22.0); after a draw
Bbox(x0=642.042, y0=523.467, x1=784.167, y1=551.467) so AnnotationBbox indeed does not know its position until a draw is called. I think OffsetBBox doesn't move the children until draw time, so in this case the text has an offset of 0, 0.

I'll maintain these are pretty low-level artists, and obviously they do not conform to the standard tight_bbox and draw behaviour. I'm not sure how far someone wants to go to fix that.

@jklymak
Copy link
Member

jklymak commented Nov 14, 2022

Confirmed same result for tight_layout. This is a long-standing behaviour.

@tacaswell
Copy link
Member

ok, so the issue is that:

  • the the location is adjusted at draw time (which makes sense given that it is by definition an offset from something else)
  • the initialized bounding box (which is going to be junk) puts it outside of the Figure
  • this confuses auto-layout tools because they are being fed bogus information
  • excluding those artists from the layout avoids the problem

I agree that this is an edge case and it may not be worth the complexity required to solve this in a general way. Maybe just a note in the AnnotationBbox docstring that it confuses the autolayout tools?

@jklymak
Copy link
Member

jklymak commented Nov 14, 2022

I think thats right. We could factor the logic that does the placement out of the draw, and call that when the extent is asked for. I think it's fair that the child doesn't know where it is, but the parent should be able to figure it out without having to do a whole draw. But as it is, draw is necessary.

We could add a comment about this until someone feels like fixing it.

@jklymak jklymak changed the title [Bug]: Constrained layout does not work well with AnnotationBbox [Bug]: AnnotationBbox does not return correct window_extent before first draw Nov 14, 2022
@jklymak jklymak added the topic: geometry manager LayoutEngine, Constrained layout, Tight layout label Nov 14, 2022
@tomicapretto
Copy link
Author

Wow, thanks a lot for the debate and the research. Really illuminating!

Feel free to do whatever you think it's better with this issue (closing, leaving open until someone fixes it, changing title, etc). This appeared when someone reported an issue in a library I wrote tomicapretto/flexitext#11 and internally we include text using AnnotationBbox

@rcomer
Copy link
Member

rcomer commented Nov 23, 2022

There seems to have been a change here for v3.6. With the code from the OP, I get

v3.5.2 (no warnings)

annotation_box_3 5

v3.6.2 (warning as in OP)
annotation_box_3 6

@QuLogic
Copy link
Member

QuLogic commented Nov 24, 2022

The warning has been around for a while, but its trigger did change slightly in #22289. I have not checked, but possibly this PR introduced the change.

@rcomer
Copy link
Member

rcomer commented Nov 24, 2022

I just had my first go with git bisect, and I think it's telling me the change was at #21935.

@jklymak
Copy link
Member

jklymak commented Nov 24, 2022

That seems like a likely place!

@rcomer rcomer added this to the v3.6.3 milestone Nov 28, 2022
@QuLogic QuLogic modified the milestones: v3.6.3, v3.7.0 Jan 11, 2023
@ksunden ksunden modified the milestones: v3.7.0, v3.7.1 Feb 14, 2023
@QuLogic QuLogic modified the milestones: v3.7.1, v3.7-doc Mar 4, 2023
@rcomer
Copy link
Member

rcomer commented Jun 24, 2023

I can no longer reproduce the warning on main, and a git bisect points to #25713. I have no clue how that changed it, but I can also report that, with v3.7.1, I see the warning with qtagg but not with agg.

Although we don't get the warning any more, we can still see from the plot that constrained layout is not being applied.
annotation_box

@rcomer
Copy link
Member

rcomer commented Jun 28, 2023

Actually, I'm starting to think the warning was a red herring: the second plot at #24453 (comment) and the one in my previous comment are consistent with constrained layout having run and made space for the annotation on the lower left of the axes. If it had not run, there would surely be more space top and right.

@rcomer rcomer modified the milestones: v3.7-doc, v3.7.2 Jul 1, 2023
@QuLogic QuLogic changed the title [Bug]: AnnotationBbox does not return correct window_extent before first draw [Bug]: AnnotationBbox does not return correct window_extent before first draw Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: confirmed bug topic: geometry manager LayoutEngine, Constrained layout, Tight layout
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants