/matplotlib

Color cycle handling #6291

Merged
merged 8 commits into from Apr 28, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
default changes
Projects
None yet
Milestone
  v2.0.0
5 participants
@mdboom @QuLogic @efiring @tacaswell @jenshnielsen
@mdboom
Owner

mdboom commented Apr 11, 2016

Replaces #5674.

This uses the color cycle in more places.

It also introduces Cn syntax for colors to directly address the Nth element in a color cycle. This was chosen over the [n] syntax in #5674 because it makes it more obvious that it's color-specific and can only be used in places where colors can be used.

mdboom added some commits Apr 11, 2016

@mdboom mdboom Fix #5577, Fix #5489: Color cycle handling 684dbf6
@mdboom mdboom Fix #5990. Scatter follows color cycle

Some checks were not successful

1 failing and 1 successful checks
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
48d5b6b

mdboom added the needs_review label Apr 11, 2016

mdboom referenced this pull request Apr 11, 2016

Closed

Improve color cycle usage #5674

mdboom added this to the 2.0 (style change major release) milestone Apr 11, 2016

@QuLogic QuLogic commented on the diff Apr 11, 2016

View full changes
examples/pylab_examples/color_demo.py
@@ -11,6 +11,9 @@
4) as a string representing a floating point number
from 0 to 1, corresponding to shades of gray.
+ 5) as a special color "Cn", where n is a number 0-9 specifying the
@QuLogic

QuLogic Apr 11, 2016

Member

The sentence above should be updated to say 5 ways.

@mdboom

mdboom Apr 11, 2016

Owner

Thanks for noticing. Done.

@QuLogic QuLogic and 1 other commented on an outdated diff Apr 11, 2016

View full outdated diff
examples/pylab_examples/color_demo.py
@@ -20,8 +23,8 @@
#subplot(111, facecolor='#ababab')
t = np.arange(0.0, 2.0, 0.01)
s = np.sin(2*np.pi*t)
-plt.plot(t, s, 'y')
-plt.xlabel('time (s)', color='r')
+plt.plot(t, s, 'C0')
+plt.xlabel('time (s)', color='C0')
@QuLogic

QuLogic Apr 11, 2016

Member

I'm pretty sure this demo is intentionally trying not to pick C0.

@mdboom

mdboom Apr 11, 2016

Owner

I'm deliberately updating it to the new recommended approach...

@QuLogic

QuLogic Apr 11, 2016

Member

Right, but I mean the original does not pick blue.

@mdboom

mdboom Apr 11, 2016

Owner

I see -- good point. I'll update.

@QuLogic QuLogic commented on the diff Apr 11, 2016

View full changes
lib/matplotlib/axes/_base.py
@@ -113,9 +116,14 @@ def _process_plot_format(fmt):
raise ValueError(
'Illegal format string "%s"; two color symbols' % fmt)
color = c
+ elif c == 'C' and i < len(chars) - 1:
+ color_cycle_number = int(chars[i + 1])
@QuLogic

QuLogic Apr 11, 2016

Member

Limited to 10 I guess, but what if you wanted to use something like Vega20 (not that we have that as a cycle at the moment)?

@mdboom

mdboom Apr 11, 2016

Owner

Yes, we should probably document that limitation. The problem with opening it up to more than 10 is that it makes backward compatibility much harder. Should C11 be the 12th color or the 2nd color with a triangle up marker? If we did allow more than one digit, we'd force people to have to write 1C1.

Since more than 10 colors is sort of an edge case, and ill-advised even then, I think it's fine to have this limitation.

@QuLogic QuLogic and 1 other commented on an outdated diff Apr 11, 2016

View full outdated diff
lib/matplotlib/colors.py
@@ -67,6 +73,19 @@
def is_color_like(c):
'Return *True* if *c* can be converted to *RGB*'
+
+ # Special-case the N-th color cycle syntax, because it's parsing
@QuLogic

QuLogic Apr 11, 2016

Member

it's -> its

@mdboom

mdboom Apr 11, 2016

Owner

Fixed.
@QuLogic
Member

QuLogic commented Apr 11, 2016

I assume the test image was not intended to have changed colours.
@mdboom
Owner

mdboom commented Apr 11, 2016

I assume the test image was not intended to have changed colours.

Indeed it was intentional. That test is testing a linestyle-only color cycle. Since it has no colors, it defaults to black, which is a more sensible default than the old blue.

mdboom added some commits Apr 11, 2016

@mdboom mdboom Fix number

All checks have failed

2 failing checks
continuous-integration/appveyor/pr — AppVeyor build failed
Details
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
649722a
@mdboom mdboom Change color used in example

All checks have failed

2 failing checks
continuous-integration/appveyor/pr — AppVeyor build failed
Details
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
3427fa7
@mdboom mdboom Fix typo

Some checks were not successful

1 failing and 1 successful checks
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
992a258
@mdboom mdboom PEP8

All checks have passed

2 successful checks
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
5626c40
@mdboom mdboom Fix Sankey example

All checks have passed

2 successful checks
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
17b71ea

@tacaswell tacaswell commented on the diff Apr 14, 2016

View full changes
lib/matplotlib/axes/_axes.py
@@ -3846,7 +3853,10 @@ def scatter(self, x, y, s=None, c=None, marker='o', cmap=None, norm=None,
if facecolors is not None:
c = facecolors
else:
- c = 'b' # The original default
+ if rcParams['_internal.classic_mode']:
+ c = 'b' # The original default
+ else:
+ c = self._get_patches_for_fill.get_next_color()
@tacaswell

tacaswell Apr 14, 2016

Owner

I am +.75 on this.

@mdboom

mdboom Apr 14, 2016

Owner

vs. c = 'C0'?

@tacaswell

tacaswell Apr 14, 2016

Owner

Yeah, that is the only other plausibly defensible position.

My only reservation is that we have pushed back on this suggestion for so long and that in most cases if you are not mapping the color you probably want to be using plot(x, y, 'o') instead.

That said, I have no real protest with this.

QuLogic added the default changes label Apr 14, 2016

@jenshnielsen jenshnielsen commented on an outdated diff Apr 25, 2016

View full outdated diff
lib/matplotlib/rcsetup.py
@@ -867,7 +867,7 @@ def validate_animation_writer_path(p):
## patch props
'patch.linewidth': [None, validate_float_or_None], # line width in points
'patch.edgecolor': ['k', validate_color], # black
- 'patch.facecolor': ['#1f77b4', validate_color], # blue (first color in color cycle)
+ 'patch.facecolor': ['C0', validate_color], # blue (first color in color cycle)
@jenshnielsen

jenshnielsen Apr 25, 2016 • edited

Owner

Delete blue in the comment here?
@efiring
Owner

efiring commented Apr 26, 2016

This is related to #6328.
@mdboom mdboom Fix comments

All checks have passed

2 successful checks
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
58e66c8

@tacaswell tacaswell merged commit 8af02b3 into matplotlib:master Apr 28, 2016
2 checks passed

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

tacaswell removed the needs_review label Apr 28, 2016

@tacaswell
Owner

tacaswell commented Apr 28, 2016

backported to v2.x as e524354

This was referenced Apr 28, 2016

Closed
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment