MNT: arghandling subplotspec

[jklymak]

In https://stackoverflow.com/questions/77710768/getting-constrained-layout-to-work-with-nested-subplots-in-matplotlib
the user did

    fig, axs = plt.subplots(1, 2)
    gs = mpl.gridspec.GridSpecFromSubplotSpec(2, 1, subplot_spec=axs[0])

which isn't crazy, though subplot_spec should really be of type SubplotSpec. Indeed it works fine if the user doesn't call constrained_layout.

This PR makes the argument a bit more forgiving and will do axs[0].get_subplotspec for the user without complaining, and will check for any other inputs not being type SubplotSpec.

[anntzer]

From the SO post, I assume the user would really also want this to auto-remove the previously existing axes, which I don't think this is something we want to do automatically?

[jklymak]

No absolutely not. OTOH you can see how they made the error, so it's easy to be a bit more forgiving.

[timhoffm]

I‘m all for making API intuitive and forgiving. However, I‘m concerned that not clearly distinguishing between subplotspec and Axes dilutes the concepts. I would be more in favor returning a helpful error message („… did you mean Axes.subplot_spec?“).

[jklymak]

We could do that instead. OTOH I wasn't suggesting we document this as working.

[timhoffm]

OTOH I wasn't suggesting we document this as working.

Uh, I see it as highly problematic to actively introduce undocumented behavior. The motivation for introducing the behavior would be that „it just works“, but that means we‘re implicitly giving API guarantees. It would be bad style to support this and take it away later by claiming „this has not been documented to work“. If we want to support something, we should document it.
More generally, people rely on observable behavior, not on documentation (c.f. Hyrum‘s Law). So, whether we document or not is not relevant for what people expect. The actual behavior is the de-facto leading measure. For clarity and consistency, we should strive to reflect that behavior in the documentation. Undocumented behavior is not a feature but either deficiency in our documentation or actually unwanted behavior.

[jklymak]

OK, fair enough. This is obscure enough that we should just error rather than documenting.

[rcomer]

Is this comment from an earlier revision? It says we allow, but then the code asserts an exception.

[jklymak]

Fixed

[QuLogic]

Any reason not to use _api.check_isinstance?

[jklymak]

I didn't realize that existed. However, that issues a generic type error, whereas we want to provide a bit of guidance here?