Hi@PiotrStrzelczykX, thanks for the PR! Can you please provide a little more context, like a link to the PR where this memory leak went fix was implemented or to an open issue, if it exists, discussing the problem you are trying to solve here?
Thanks!

Hello@story645, I added link to PR which changed this behavior.
I didn't prepare issue, because fix is so obvious.
I've spotted a issue after upgrading matplotlib:

AttributeError: 'tuple' object has no attribute 'get_figure' Error_place:  File 'C:\\Python310\\lib\\site-packages\\matplotlib\\colorbar.py':1446 in make_axes  fig = parents[0].get_figure()

and prepared this fix.

Please add a test and maybe also include your test code in the issue. If we broke something once it is because there is no test.

Actually I guess we should discuss if we want to accept a tuple. That may have worked before, but it wasn't documented, or particularly intended.

General typing advice is to be open about what you accept and strict about what you output.

parents is used to a) getparents[0] and b) loop over (twice)

Perhaps we should be testing/documenting ascollections.abc.Sequence instead (which would get both list and tuple, as well as anything else which quacks like we use it)?

Sure that sounds reasonable. However, being permissive is tricky practically when you are trying to write type checks etc, and for testing, since we can't check everything that quacks to make sure we don't break it.

I suspect the right check here is to see if it is an iterable and special case if it is an array. Maybe right path is something like

if is_iterable(parents):    if isinstance(parents, np.array):         list(parents.flat)    else:        parents = list(parents)else:    parents = [parents]

thus we come out of this block it is definitly a list, we special case the np.array case (which might break with some future numpy-clone), and we let Python do the coerces to list for us.

@tacaswell checkingis_iterable(parents) is not firm, as there is alsoparents[0] access, checkingis_instance(parents, collections.abc.Sequence) (suggested by@ksunden) would be better.

Please add a test and maybe also include your test code in the issue. If we broke something once it is because there is no test.

@jklymak test added.

I not insist on this change -- I thought that it may be useful for many users so I push it, but if you think it may be incompatible just discard it.

I think this description inFigure.colorbar should also be updated for consistecy

@tacaswell’s suggestion of converting any iterable to a list would allow the user to passdict.values(), wheredict comes fromsubplot_mosaic. That seems worth having to me.

I think we should actually figure out a consistent way to deal with itterables and perhaps factor it into_api so that we do it consistently in different places. This came up recently in another PR as well (sorry I forget which one)

I wonder if co-ercing to numpy array first is a good consistent way to do this? Then we are just relying on their broadcasting rules, which is easy to explain. egparents = list(np.asarray(parents).flat). Or we could even just leave it as an array?

list() does the right thing for dictionaries, whilenp.asarray does not:

In [3]: d = dict(a=1, b=2)In [4]: import numpy as npIn [5]: np.asarray(d.values())Out[5]: array(dict_values([1, 2]), dtype=object)In [6]: list(d.values())Out[6]: [1, 2]

@tacaswell checkingis_iterable(parents) is not firm, as there is alsoparents[0] access, checkingis_instance(parents, collections.abc.Sequence) (suggested by@ksunden) would be better.

The code provided by@tacaswell enforces that it is alist (which can be indexed) whenparents[0] is called, so it is perfectly fine for any (finite) iterable. So that code is in fact more general, and will accept more inputs as valid.

@PiotrStrzelczykX, thank you for this PR. In fact, I tripped over this (not accepting a tuple argument) myself a week or two ago. We will be happy to accept the PReither as-is,or after one more change, if you choose to make it: specifically, the suggestion by@tacaswell, and endorsed by@rcomer and myself, to broaden the supported types to include iterables, likedict.values(). Fleshed out, it becomes

ifnp.iterable(parents):ifisinstance(parents,np.ndarray):parents=list(parents.flat)else:parents=list(parents)else:parents= [parents]

If you do choose to make this change, then where you have "sequence" in the docstring it would become the more general "iterable".

It is possible that some time later, after merging the PR, we might factor this argument-handling functionality out for use in other places; but we don't want to stall the PR while hashing out larger questions of API design and factoring.

Also, if you choose to make the change, you could add a line to the test:

fig.colorbar(im,ax=dict(a=ax[0],b=ax[1]).values())

I just noticed that the constrained layout guide states

If you specify a list of axes (or other iterable container) to the ax argument of colorbar, constrained_layout will take space from the specified axes.

https://matplotlib.org/stable/tutorials/intermediate/constrainedlayout_guide.html#colorbars

@PiotrStrzelczykX could you let us know if you intend to take this any further? As@efiring said, we are happy to merge as is. However, it would be better not to merge yet if you are planning to push further changes.

I'm going to merge this on my approval and the comments from@efiring and@rcomer and then open up a follow on PR with the additional changes.

Thank you for your work on this@PiotrStrzelczykX and congratulations on your first merged Matplotlib PR 🎉 I hope we hear from you again!