-
-
Notifications
You must be signed in to change notification settings - Fork 8k
Deprecate color keyword argument in scatter #4675
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
Conversation
👍
Also removed uses of the color keyword argument in examples, replacing it with c.
Can you add a note in api_changes?
Done.
Where the color
kwargs comes from in Artist
all the way at the bottom of the call stack (any kwargs which make it to the Artist.__init__
method are used to search for self.set_*
methods which are then called with the value. In that sense we can't really deprecate the use of color
as a kwargs.
@tacaswell Then perhaps just change the warning to a different type and inform the user that it is discouraged?
Hopefully this is something that traitlets will help identify and clean up.
The c
keyword argument is grim. The number of times that I've been asked "what do 's' and 'c' do?"...
Maybe we are looking at this the wrong way?
mwaskom
commented
Nov 10, 2015
If possible, I'd like to ask that you please not remove color
from scatter
. Doing would disrupt higher level functions that are written against a general API and assume that passing color
to a lower-level matplotlib function will do something useful (i.e., FacetGrid
in seaborn). I also agree that from a user perspective, it would be confusing for color
to work in many functions but not in scatter
.
Does anyone want to resurrect this? I suspect its still a problem. Note that the deprecation should use our deprecation function....
mwaskom
commented
Aug 17, 2018
I guess I’d repeat my request that matplotlib not act to make the API less consistent.
I think the issue is that color
is ambiguous, whereas facecolor
and edgecolor
are not. If we are to keep color, then I think we need to agree on a meaning of it across objects that have both face/edgecolor
. I don't particuarly care myself - I'd say c/color
is what you start with for both, and face/edgecolor
override. If thats whats already done, then maybe this should just be closed, or reduced to documenting color
.
mwaskom
commented
Aug 17, 2018
I think the issue is that color is ambiguous, whereas facecolor and edgecolor are not. If we are to keep color, then I think we need to agree on a meaning of it across objects that have both face/edgecolor.
I don't believe that's the issue here. The color
vs facecolor
/edgecolor
distinction is present in several matplotlib functions and should not be handled only by deprecating color
in scatter
. Were we to discuss that issue I would also stress the benefits of having all matplotlib functions accept and use color
. But the issue here is about the specific confusion of having c
and color
. While I appreciate how having multiple ways to specify the color is confusing, I don't think the best solution is the remove the way that works everywhere else in matplotlib.
Let me rephrase. The fundamental issue is that color
is ambiguous. I tend to agree with you that removing it is not correct. However, what to do with it is the question, and how to fix the documentation?
mwaskom
commented
Aug 17, 2018
color
changes the color of the could of points uniformly. c
changes the color of each point independently. You can also provide a single color to c
and it will "broadcast" to all the points.
So would it be possible to make c
an alias of color
(or vice versa); and possily the same with s
and size
? Meaning you can specify either of them (but not both) and see the same plot.
I suppose there can't be any use case where both are needed simultaneously. The only issue may be that color accepts an rbg color like color=[0.,0.,1.]
while c
may not (at least in the case you have 3 scatter points. Or are there other differences?
mwaskom
commented
Aug 17, 2018
The behavior you describe already exists:
plt.scatter([0, 1], [0, 1], c="red", color="blue")
--------------------------------------------------------------------------- ValueError Traceback (most recent call last) <ipython-input-31-7bf4d623a32e> in <module>() ----> 1 plt.scatter([0, 1], [0, 1], c="red", color="blue") ~/anaconda/lib/python3.6/site-packages/matplotlib/pyplot.py in scatter(x, y, s, c, marker, cmap, norm, vmin, vmax, alpha, linewidths, verts, edgecolors, hold, data, **kwargs) 3468 vmin=vmin, vmax=vmax, alpha=alpha, 3469 linewidths=linewidths, verts=verts, -> 3470 edgecolors=edgecolors, data=data, **kwargs) 3471 finally: 3472 ax._hold = washold ~/anaconda/lib/python3.6/site-packages/matplotlib/__init__.py in inner(ax, *args, **kwargs) 1853 "the Matplotlib list!)" % (label_namer, func.__name__), 1854 RuntimeWarning, stacklevel=2) -> 1855 return func(ax, *args, **kwargs) 1856 1857 inner.__doc__ = _add_data_doc(inner.__doc__, ~/anaconda/lib/python3.6/site-packages/matplotlib/axes/_axes.py in scatter(self, x, y, s, c, marker, cmap, norm, vmin, vmax, alpha, linewidths, verts, edgecolors, **kwargs) 4213 facecolors = co 4214 if c is not None: -> 4215 raise ValueError("Supply a 'c' kwarg or a 'color' kwarg" 4216 " but not both; they differ but" 4217 " their functionalities overlap.") ValueError: Supply a 'c' kwarg or a 'color' kwarg but not both; they differ but their functionalities overlap.
Although that message could probably be a bit more specific...
Exactly. "they differ but their functionalities overlap".
I'm asking to not let them differ (=alias), or at least work towards not letting them differ if that breaks existing functionality.
mwaskom
commented
Aug 17, 2018
Feel free to pursue that; I will disengage from the conversation.
I think that it's nice to be able to pass an RGB tuple to color
and have it do the right thing even with 3 points — the change to how c
behaves in that circumstance caused me and others a lot of headaches.
My only aim here has been to strongly encourage matplotlib not to remove the ability to pass a normal color specification to color=
and I think I've made that point as well as I can.
As a rule I think api changes and deprecation are not good first issues.
OK I'm closing this because it doesn't really seem to be an urgent issue, and maybe we need to figure out library wide how to handle color
, edgecolor
, facecolor
. Feel free to re-open.
There is a comment in
Axes.scatter
stating "'color' should be deprecated in scatter, or clearly defined; since it isn't, I am giving it low priority.". It is neither defined in the documentation nor deprecated, so this adds a deprecated warning to be more explicit about it.