-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
scatter could not raise when colors are provided but position data are empty #14113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Thanks for the notification. What is the purpose of specifying 3 colors? |
One particular context (in the linked issue) is following a principle in seaborn where categorical plots represent "empty" categories if they are specified but not present in (a subset of) the data. |
Another use case is that seaborn often calls the plotting function it's going to use with empty data to hook into the internal matplotlib cycler while still honoring the user's kwargs. But this new change triggers an exception when that happens in the |
Can you please provide one or two seaborn code examples so that I can see how that is used in real life and track it down? |
Requiring that all of the color and size handling be done when there is no position data seems like a stretch too far to be required of mpl. Since you are making a dummy plot, extracting info from it, and discarding it, and since you know how many values you are providing in your "c" argument, I suggest that you simply provide dummy x and y arrays to match. |
OK fair enough, I don't have time to argue about this. I predicted that this would be a waste of time, and it was. |
Shouldn‘t we for now take the burden of creating suitable nan-valued x/y for this special case to prevent broken seaborn versions out in the field? While empty data is IMO not a valid use case, the only way to work around this for the end user is to install an older version of matplotlib; and that‘s also not what we want. That functionality would be deprecated in 3.2 and removed eventually, but it would give time to roll out a new seaborn version that uses dummy data itself. Reopening and marking as release-critical for 3.1, because if we want to do this, we should bring it still in 3.1. @tacaswell I think it‘s up to you to decide. |
Nan-valued? Do you mean to put a block of code in the beginning that checks for empty x and y but non-empty c, and then makes nan-filled x and y with lengths matching the first dimension of c? Looking at the seaborn code that @mwaskom pointed to, I think it might have to also check for non-empty s and do the same thing. And effectively, facecolor and facecolors and color also have to be checked, so probably the block has to be a way down in the Do you really want to try to jam all this into 3.1, with tests and deprecation warnings? I don't. |
I assume that it should be fairly straigt forward to add a block
at matplotlib/lib/matplotlib/axes/_axes.py Line 4421 in 983bbdd
Untested, but I don't expect it to be much more than this. Yes, I would want to jam this into 3.1. The scope of the change is fairly limited. I wouldn't be worried much that it breaks something else (of course needs tests). And it solves a real world issue out there. Deprecation would just start with 3.2, so that a new seaborn can be out before 3.2 is released. But of course, that's all for @tacaswell to finally decide. |
My vote is w/ @efiring on this. I completely empathize with folks being busy, but this seems like a work around that seaborn happened to be making use of rather than something we actually want, and its in their court to fix it. Note that this is a change introduced in 3.0; if this was in 3.1rc I'd feel a bit differently. I think we need to strongly encourage downstream projects to test off our master branch and squak if we break something so we can start a dialogue well before the release cycle about whether a change is a good idea or not. I think cartopy and yt do this, and its been very useful to catch things we hadn't thought of. |
The 3.0.x series added new checks in
scatter
for equivalence between the length of thex
,y
vectors and the length of thec
vectors.These changes have broken code in seaborn that sometimes calls scatter with empty position data but non-empty color data, e.g.
IMO this is an unnecessary exception because it conflicts with another patten in matplotlib, which is passing empty data along with specified style attributes (e.g. to create a proxy artist). I appreciate that the
c
parameter occupies a fuzzy zone between data and style attribute. But specifically in the case when the position data are empty, no points will have the wrong colors.The text was updated successfully, but these errors were encountered: