TYP: Add common-type overloads of subplot_mosaic

[QuLogic]

PR summary

I'll assert, without proof, that passing a single string, a mosaic list of strings, or a mosaic list all of the same type is more common than passing arbitrary unrelated hashables. Thus it is somewhat convenient if the return type stipulates that the resulting dictionary is also keyed with strings or the common type.

This also fixes the type of the per_subplot_kw argument, which also allows dictionary keys of tuples of the entries.

I don't like that we need a few ignores, but this does work from an external point of view:

from typing import TYPE_CHECKING

import matplotlib.pyplot as plt

if not TYPE_CHECKING:
    reveal_type = print

fig0, ax0 = plt.subplot_mosaic('ab;cd')
reveal_type(ax0)  # Should be str keys

fig1, ax1 = plt.subplot_mosaic([['a', 'b'], ['c', 'd']])
reveal_type(ax1)  # Should be str keys

fig2, ax2 = plt.subplot_mosaic([[0, 1], [2, 3]])
reveal_type(ax2)  # Should be int keys

fig3, ax3 = plt.subplot_mosaic([['a', 'b'], ['c', 'd'], [0, 1]])
reveal_type(ax3)  # Should be any Hashable keys

fig = plt.figure()

ax4 = fig.subplot_mosaic('ab;cd')
reveal_type(ax4)  # Should be str keys

ax5 = fig.subplot_mosaic([['a', 'b'], ['c', 'd']])
reveal_type(ax5)  # Should be str keys

ax6 = fig.subplot_mosaic([[0, 1], [2, 3]])
reveal_type(ax6)  # Should be int keys

ax7 = fig.subplot_mosaic([['a', 'b'], ['c', 'd'], [0, 1]])
reveal_type(ax7)  # Should be any Hashable keys

produces:

foo.py:9: note: Revealed type is "builtins.dict[builtins.str, matplotlib.axes._axes.Axes]"
foo.py:12: note: Revealed type is "builtins.dict[builtins.str, matplotlib.axes._axes.Axes]"
foo.py:15: note: Revealed type is "builtins.dict[builtins.int, matplotlib.axes._axes.Axes]"
foo.py:18: note: Revealed type is "builtins.dict[typing.Hashable, matplotlib.axes._axes.Axes]"
foo.py:23: note: Revealed type is "builtins.dict[builtins.str, matplotlib.axes._axes.Axes]"
foo.py:26: note: Revealed type is "builtins.dict[builtins.str, matplotlib.axes._axes.Axes]"
foo.py:29: note: Revealed type is "builtins.dict[builtins.int, matplotlib.axes._axes.Axes]"
foo.py:32: note: Revealed type is "builtins.dict[typing.Hashable, matplotlib.axes._axes.Axes]"
Success: no issues found in 1 source file

i.e., for any common-types inputs, the dictionary uses that type as key, whereas mixed types is a generic Hashable.

PR checklist

• [n/a] "closes #0000" is in the body of the PR description to link the related issue
• new and changed code is tested
• [n/a] Plotting related features are demonstrated in an example
• [n/a] New Features and API Changes are noted with a directive and release note
• [n/a] Documentation complies with general and docstring guidelines

[QuLogic]

Looking over mypy results, this introduces a few errors for test_figure.py, because it now requires 2D lists and some tests check that 1D lists will error (so this is expected.)

It also shows that a mosaic with a nested 2D list can't catch the overload, even if all contained items are str. This is because mixing str with list[list[str]] as list element makes mypy infer that it is overall a list[object]. It's unfortunate that we can't catch this case, but the problem is on the caller side.

I think if callers want to be explicit, they could annotate their mosaic as list[HashableList[str]]. It might be a bit annoying to have list appear twice there; maybe we should have a MosaicList = list[HashableList] sort of alias?

[QuLogic]

I also checked over the galleries, and it fixes a few issues like:

fig, axd = plt.subplot_mosaic(...)
for title, ax in axd.items():
    ax.set_title(title)  # Previously failed as set_title didn't take a Hashable.

However, I see two issues:

1. If you save the mosaic to a variable, it appears to become list[list[str]], and IIUC, list is invariant and doesn't match list[Hashable[list]] any more (whereas the literal was allowed a bit more leeway).
2. One example uses a 2D NumPy array instead of a list of lists.

I'm not sure if we should be using a Sequence instead of list as the basis of HashableList.

[ksunden]

This is failing the doc build because of the changes to HashableList

[ksunden]

Well it looks like you sorted the Hashable list sphinx problem, but sphinx introduced new problems, PR Submitted upstream

[ksunden]

Type hints can be removed from the implementation signature with overloads

they just clutter everything up trying to encompass all possible cases, and don't actually add anything that is not covered by the extant type hints

The examples from PEP 484 and the typing module docs omit the type hints on the actual implementation.

[ksunden]

(This may allow getting rid of the type ignores? not sure)

[QuLogic]

If I drop all the types on the function, then mypy --strict lib/matplotlib gives:

lib/matplotlib/pyplot.py:1652: error: Function is missing a type annotation  [no-untyped-def]

and all the type ignores are no longer needed, but that's because the function's untyped, no? It is fine without --strict though.

[ksunden]

That is annoying, I don't wholly agree with that, looks like the reasoning is that it won't type check the implementation then, but I'm not too concerned about that...

I could go either way.

[ksunden]

I think it can be improved by removing the Hashable path (and making the type var have bound=Hashable in there... may actually be able to get rid of str from all but the mosaic arg...

Actually its not clear to me what HashableList[Hashable] is doing for us at all, shouldn't that just be the same as HashableList, just not having the typevar bound to any particular type?

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout v3.8.x
git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 b71901283457c9cb3cc0a6fc0261a2bba729c823

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #26491: TYP: Add common-type overloads of subplot_mosaic'

4. Push to a named branch:

git push YOURFORK v3.8.x:auto-backport-of-pr-26491-on-v3.8.x

5. Create a PR against branch v3.8.x, I would have named this PR:

"Backport PR #26491 on branch v3.8.x (TYP: Add common-type overloads of subplot_mosaic)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.