Skip to content
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

Scatter "c" kwarg hsa changed #5141

Closed
pelson opened this issue Sep 25, 2015 · 5 comments
Closed

Scatter "c" kwarg hsa changed #5141

pelson opened this issue Sep 25, 2015 · 5 comments
Assignees
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Milestone

Comments

@pelson
Copy link
Member

pelson commented Sep 25, 2015

In previous releases, it was possible, when using scatter, to colour makers based on your data directly with a scatter call:

import numpy as np
import matplotlib.pyplot as plt


x = np.linspace(0, 10, 20)
y = np.linspace(-5, 5, 10)
x, y = np.meshgrid(x, y)
data = x * y

plt.scatter(x, y, c=data)
plt.show()

In mpl 1.4:
figure_1

In mpl 1.5:

ValueError: to_rgba: Invalid rgba arg "[ -0.          -2.63157895  -5.26315789  -7.89473684 -10.52631579
 -13.15789474 -15.78947368 -18.42105263 -21.05263158 -23.68421053
 -26.31578947 -28.94736842 -31.57894737 -34.21052632 -36.84210526
 -39.47368421 -42.10526316 -44.73684211 -47.36842105 -50.        ]"
length of rgba sequence should be either 3 or 4

Haven't had a chance to dig into exactly what has changed, but 18cc8bf looks like it could be related.

Ping @efiring just incase this was intentional.

@dopplershift
Copy link
Contributor

That would be a pretty major API breakage if intentional. How did we not have a test of that case?

@tacaswell
Copy link
Member

It looks like the logic has become a bit crossed.

In https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/axes/_axes.py#L3832 we ravel the x and y, but then down at https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/axes/_axes.py#L3845 we check if c and x are the same shape to decide if we should ravel c.

Definitely a bug.

@tacaswell tacaswell added this to the next point release (1.5.0) milestone Sep 25, 2015
@tacaswell tacaswell added the Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. label Sep 25, 2015
tacaswell added a commit to tacaswell/matplotlib that referenced this issue Sep 25, 2015
@tacaswell tacaswell self-assigned this Sep 25, 2015
@efiring
Copy link
Member

efiring commented Sep 25, 2015

@pelson, it's fortunate you found this--it's quite a nasty bug that I caused. @dopplershift, it's easy to not have a test; we handle all sorts of combinations of inputs, and only a tiny fraction of those combinations gets tested. I suspect most of our tests now are for corner cases where bugs have turned up, rather than being a systematic sweep through mpl's capabilities.

@dopplershift
Copy link
Contributor

I guess I'm more surprised that such a basic part of scatter's functionality wasn't tested via collateral damage from "corner case" tests.

@pelson
Copy link
Member Author

pelson commented Sep 28, 2015

Super. Thanks @tacaswell & @efiring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

No branches or pull requests

4 participants