FIX: ignore invisible gridliner labels in title adjustment

[rcomer]

Rationale

Since matplotlib/matplotlib#31285, Text objects return a null bbox if they are invisible or have an empty string. This broke our title positioning logic, because we specifically look at the ymax of the Bbox rather than taking box unions.

The invalid title position gets incorporated into the axes tight bbox, which also becomes invalid and therefore messes up constrained layout and we get a plot like #2666 (comment) where the titles have disappeared and the gridline labels overlap each other or go off the side of the plot.

This can all be avoided if we ignore the invisible or empty labels. Running the relevant test locally with this branch and Elliot's branch from matplotlib/matplotlib#31521, I got a sensible image

Implications

This clearly won't help anyone who uses existing Cartopy releases with the new Matplotlib release.

[greglucas]

This seems like a good thing to do anyways, but I'm wondering if we want to do the bbox union instead of max like you mentioned just below this to be more robust there too? That can be a future PR too, feel free to merge as is.

[rcomer]

Looks like it is faster to only consider the ymax, although I only tried with one example. Also most of the slowdown came from having the null bboxes in the union.

In [22]: def get_top():
    ...:     top = -1
    ...:     for label in (gl.top_label_artists +
    ...:                   gl.geo_label_artists):
    ...:         if not label.get_visible() or label.get_text() == "":
    ...:             continue
    ...:         bb = label.get_tightbbox(renderer)
    ...:         top = max(top, bb.ymax)
    ...: 

In [23]: def get_top2():
    ...:     bbs = []
    ...:     for label in (gl.top_label_artists +
    ...:                   gl.geo_label_artists):
    ...:         bbs.append(label.get_tightbbox(renderer))
    ...:         bb = mtransforms.Bbox.union(bbs)
    ...:         top = bb.ymax
    ...: 

In [24]: def get_top3():
    ...:     bbs = []
    ...:     for label in (gl.top_label_artists +
    ...:                   gl.geo_label_artists):
    ...:         if not label.get_visible() or label.get_text() == "":
    ...:             continue
    ...:         bbs.append(label.get_tightbbox(renderer))
    ...:         bb = mtransforms.Bbox.union(bbs)
    ...:         top = bb.ymax
    ...: 

In [25]: %timeit get_top()
54.3 μs ± 113 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

In [26]: %timeit get_top2()
228 μs ± 667 ns per loop (mean ± std. dev. of 7 runs, 1,000 loops each)

In [27]: %timeit get_top3()
74.2 μs ± 165 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

[QuLogic]

...: bbs.append(label.get_tightbbox(renderer))
...: bb = mtransforms.Bbox.union(bbs)

Should you not be doing the union once at the end instead of each time in the loop?

[rcomer]

...: bbs.append(label.get_tightbbox(renderer))
...: bb = mtransforms.Bbox.union(bbs)

Should you not be doing the union once at the end instead of each time in the loop?

oh yes 🤦‍♀️

[rcomer]

I think I did it right this time, and only considering the ymax still comes out fastest, though these numbers are not large in the scheme of things: drawing the whole figure from the test with draw_without_rendering takes nearly 2 seconds on my laptop!

In [23]: def get_top():
    ...:     top = -1
    ...:     for label in (gl.top_label_artists +
    ...:                   gl.geo_label_artists):
    ...:         if not label.get_visible() or label.get_text() == "":
    ...:             continue
    ...:         bb = label.get_tightbbox(renderer)
    ...:         top = max(top, bb.ymax)
    ...: 

In [24]: def get_top2():
    ...:     bbs = []
    ...:     for label in (gl.top_label_artists +
    ...:                   gl.geo_label_artists):
    ...:         bbs.append(label.get_tightbbox(renderer))
    ...:     bb = mtransforms.Bbox.union(bbs)
    ...:     top = bb.ymax
    ...: 

In [25]: def get_top3():
    ...:     bbs = []
    ...:     for label in (gl.top_label_artists +
    ...:                   gl.geo_label_artists):
    ...:         if not label.get_visible() or label.get_text() == "":
    ...:             continue
    ...:         bbs.append(label.get_tightbbox(renderer))
    ...:     bb = mtransforms.Bbox.union(bbs)
    ...:     top = bb.ymax
    ...: 

In [26]: %timeit get_top()
79.4 μs ± 699 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

In [27]: %timeit get_top2()
165 μs ± 524 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

In [28]: %timeit get_top3()
110 μs ± 565 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

In [29]: %timeit fig.draw_without_rendering()
1.92 s ± 21.1 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)