@greglucas says:

Thanks for the heads up. Note that this isn't targeted at 3.7, so we do have some time to think about the best approach here.

I guess we could warn whenever get_facecolors() or get_array() would return a flattened array. But there wouldn't be a way to easily suppress the warning and get the new behavior without a temporary keyword argument which would then also require removal downstream later.

I agree, we should probably do some kind of warning/compatibility helper here.

Agree something needs to be done. However, I assume that keeping the 2D input structure is preferable to the flattening for downstream libraries (@mwaskom?), and it's just the type instability that is the problem? Or is flattening preferred for some reason?

That's right, the logic of having the artist data match the input seems totally reasonable to me, it's just the change that's going to cause problems.

I think we usually try and do the dance as follows: add a flatten kwarg to pcolormesh:

3.8: default flatten=True, but warn explicit flatten=False/True would not warn.
3.10 default flatten=False. deprecate kwarg
3.12 remove flatten kwarg?

If we want, flatten could just be applied on access versus internally.

It's kind of a pain for everybody over 3_ years, but I expect this is mostly for downstream libraries who would eventually benefit?

Discussed on the call this week.

We decided that methods like get_facecolor should gain a flatten kwarg, and just default to True to preserve the old behaviour. If someone wants the shaped arrays they can set this to False. We will do this with no plan to change the default, but just offer flatten=False as a service. Somewhat suboptimal (surely you want the shaped arrays!), but no deprecation dance, and most users accessing this information will be downstream developers and/or relatively sophisticated.

If there are any bare properties that folks are accessing we should make sure there are set_/get_ methods that have flatten kwargs, to gently discourage direct access. (We could also add getter/setter methods to them if desired).

@mwaskom will this work for seaborn?

Yeah, this sounds like a reasonable stability-prioritizing approach to me. Thanks!

This is not fully backward compatible, unfortunately :( set_array() does not auto-flatten inputs currently, so there is an inconsistency with getting quantities out of the collection before/after setting the array data if we force-flatten for users.

import matplotlib.pyplot as plt
import numpy as np

fig = plt.figure()

x = np.arange(5)
X, Y = np.meshgrid(x, x)

mesh = plt.pcolormesh(X, Y, X*Y)
fig.draw_without_rendering()

print("Before set_array()")
print(mesh.get_array().shape)
print(mesh.get_facecolors().shape)
mesh.set_array(X*Y)
fig.draw_without_rendering()
print("After set_array()")
print(mesh.get_array().shape)
print(mesh.get_facecolors().shape)

MPL v3.6

Before set_array()
(25,)
(25, 4)
After set_array()
(5, 5)
(5, 5, 4)

MPL main

Before set_array()
(5, 5)
(5, 5, 4)
After set_array()
(5, 5)
(5, 5, 4)

That is .... special? I guess we can maintain backcompat by carrying a private property around as to whether the values were set by a setter or not, but thats not pretty...

As a general rule, this is why its good for the init classes to use the public setters if at all possible ;-)...

I thought a bit more about this and I think downstream libraries are all failing on the get_facecolor() and get_edgecolor() comparisons, and not the get_array()? I think the main thing we want is for the data array to be aligned with the mesh shape and not ravel()ed. But, I think we can still say that all Collections will return flattened paths/colors and that is probably OK. So, I'd propose:

mesh.get_array(): (n, m)
mesh.get_facecolor(): (n*m)
mesh.get_edgecolor(): (n*m)
I think these are the only mapped properties (?), so that seems reasonable to me for backwards compatibility. The only change here would be that get_array() will always return the 2D version now, whereas before it would depend on whether you'd called set_array() before or not.

I have that implementation in a commit in PR #25027 which seems like a reasonable alignment to me. Thoughts from others on that?

I have that implementation in a commit in PR #25027 which seems like a reasonable alignment to me. Thoughts from others on that?

Since nobody said "no" to this, we could probably link the PR?

I'm going to close this as completed because I think we are back to the previous edgecolor and facecolor behavior. @mwaskom are you able to confirm that the regression you pointed out in #24638 (comment) is back to what you expect in seaborn?