Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Closed
jhamrick wants to merge 3 commits into matplotlib:master from jhamrick:deprecate-color

Conversation

Copy link
Contributor

@jhamrick jhamrick commented Jul 13, 2015

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.

Copy link
Member

👍

Copy link
Contributor Author

Also removed uses of the color keyword argument in examples, replacing it with c.

Copy link
Member

Can you add a note in api_changes?

@tacaswell tacaswell added this to the next point release milestone Jul 13, 2015
Copy link
Contributor Author

Done.

Copy link
Member

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 tacaswell modified the milestones: next point release, proposed next point release Jul 14, 2015
Copy link
Member

@tacaswell Then perhaps just change the warning to a different type and inform the user that it is discouraged?

Copy link
Member

Hopefully this is something that traitlets will help identify and clean up.

Copy link
Member

pelson commented Sep 25, 2015

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?

Copy link

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.

@tacaswell tacaswell modified the milestones: 2.1 (next point release), 2.2 (next next feature release) Sep 24, 2017
@jklymak jklymak modified the milestones: needs sorting, v3.0 May 9, 2018
@tacaswell tacaswell modified the milestones: v3.0, v3.1 Jul 9, 2018
Copy link
Member

jklymak commented Aug 17, 2018

Does anyone want to resurrect this? I suspect its still a problem. Note that the deprecation should use our deprecation function....

@jklymak jklymak added Good first issue Open a pull request against these issues if there are no active ones! and removed status: needs review labels Aug 17, 2018
Copy link

mwaskom commented Aug 17, 2018

I guess I’d repeat my request that matplotlib not act to make the API less consistent.

jklymak reacted with thumbs up emoji

Copy link
Member

jklymak 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 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.

timhoffm reacted with thumbs up emoji

Copy link

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.

Copy link
Member

jklymak commented Aug 17, 2018

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?

Copy link

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.

Copy link
Member

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?

Copy link

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...

Copy link
Member

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.

Copy link

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.

@tacaswell tacaswell added API: changes and removed Good first issue Open a pull request against these issues if there are no active ones! labels Aug 17, 2018
Copy link
Member

As a rule I think api changes and deprecation are not good first issues.

Copy link
Member

jklymak commented Oct 6, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
v3.1.0
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /