VIS: Allow 'C0'-like plotting for plotting colors #15516 #15873
Conversation
doc/source/whatsnew/v0.20.0.txt
Outdated
| @@ -177,6 +177,7 @@ Other enhancements | |||
| - The ``usecols`` argument in ``pd.read_csv`` now accepts a callable function as a value (:issue:`14154`) | |||
| - The ``skiprows`` argument in ``pd.read_csv`` now accepts a callable function as a value (:issue:`10882`) | |||
| - ``pd.DataFrame.plot`` now prints a title above each subplot if ``suplots=True`` and ``title`` is a list of strings (:issue:`14753`) | |||
| - ``pd.DataFrame.plot`` can pass matplotlib 2.0 default color cycle as a single string as color parameter (:issue:`15516`) | |||
There was a problem hiding this comment.
can you put a pointer into mpl docs (or pandas docs) about these color.
pandas/tests/plotting/test_frame.py
Outdated
| @@ -141,6 +141,12 @@ def test_plot(self): | |||
| result = ax.get_axes() # deprecated | |||
| self.assertIs(result, axes[0]) | |||
|
|
|||
| def test_mpl2_color_cycle_str(self): | |||
| colors = ['C' + str(x) for x in range(10)] | |||
There was a problem hiding this comment.
can you add the issue number as a comment
| msg = ("'{0}' can be parsed as both single color and " | ||
| "color cycle. Specify each color using a list " | ||
| "like ['{0}'] or {1}") | ||
| raise ValueError(msg.format(colors, list(colors))) |
There was a problem hiding this comment.
comment here about what you are doing
pandas/tools/plotting.py
Outdated
| "color cycle. Specify each color using a list " | ||
| "like ['{0}'] or {1}") | ||
| raise ValueError(msg.format(colors, list(colors))) | ||
| if color[0] == 'C' and len(color) == 2: |
There was a problem hiding this comment.
this could in theory break if color=''?
There was a problem hiding this comment.
This causes a ZeroDivisionError when trying to handle cases where there are more lines than specified colors (or vice versa). Probably can add a try/except block here
In matplotlib, passing color='' causes ValueError: Invalid RGBA argument: ''. I could add a more meaningful error message for this case as part of this PR.
Codecov Report
@@ Coverage Diff @@
## master #15873 +/- ##
==========================================
+ Coverage 90.99% 90.99% +<.01%
==========================================
Files 143 143
Lines 49479 49481 +2
==========================================
+ Hits 45021 45023 +2
Misses 4458 4458
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #15873 +/- ##
==========================================
+ Coverage 90.95% 91.03% +0.07%
==========================================
Files 145 145
Lines 49534 49582 +48
==========================================
+ Hits 45056 45136 +80
+ Misses 4478 4446 -32
Continue to review full report at Codecov.
|
pandas/tools/plotting.py
Outdated
| # for supporting matplotlib < 2.0.0 | ||
| if re.match(r'C[0-9]', colors) and len(colors) == 2: | ||
| prop_cycler = plt.rcParams['axes.prop_cycle'] | ||
| hex_color = prop_cycler.by_key().get('color', ['k']) |
There was a problem hiding this comment.
I think this prop_cycler is also only supported for matplotlib versions >= 1.5?
See here how it is handled at another location: https://github.com/GuessWhoSamFoo/pandas/blob/e2497222ecf64e07f4bd7873a1b860021b0b605c/pandas/tools/plotting.py#L185-L190
|
In order to pass tests on matplotlib 1.5, converting C[0-9] to a hex code works, but the tests are not as meaningful since there are already tests for that. |
|
Maybe we shouldn't try to support matplotlib versions < 2.0.0 ? It is only a feature from that version? The we also don't have the problem of failing tests on older matplotlib version (as the test could just be skipped for those) |
pandas/tests/plotting/test_frame.py
Outdated
| for c in list(plt.rcParams['axes.prop_cycle'])] | ||
| df = DataFrame(randn(10, 3), columns=['a', 'b', 'c']) | ||
| for c in colors: | ||
| _check_plot_works(df.plot, color=hex_color[int(c[1])]) |
There was a problem hiding this comment.
You should test here with the actual 'C0', .. instead of with the converted to hex colors ?
pandas/tests/plotting/test_frame.py
Outdated
| for c in colors: | ||
| _check_plot_works(df.plot, color=hex_color[int(c[1])]) | ||
| with tm.assertRaises(ValueError): | ||
| df.plot(color='') |
There was a problem hiding this comment.
Can you put this in a separate test? (just give it another name, it is not really related to mpl2)
|
Fixed test and figured out how to test only for matplotlib >= 2.0.0 The test for There seems to be no remaining cases where a string passed as a color can also be parsed as an additional color outside of |
|
@GuessWhoSamFoo thanks! |
git diff upstream/master --name-only -- '*.py' | flake8 --diffDefined special case for 'CN' cases where N is 0 to 9. There may be custom color cycles that can also be parsed as a single color so the ValueError message is left intact (unless this is false).