Make color validation more forgiving

[AllenDowney]

The current version throws a false positive if the style string contains s or o, which are valid marker styles and not colors. For example:

style='s', color='C3'

should be legal, but currently throws an error.

My suggestion is to check for only the letters that are color codes.

• closes #xxxx
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[WillAyd]

Can you add a test for this? I'm not super familiar with this code but the intent of the ValueError getting raises also doesn't seem to match what you are asking for here, so test would help clarify that

[AllenDowney]

I know that in theory it is best for the person submitting a PR to write tests, but I have almost no familiarity with the code base, so it would take me much longer than someone who knows the code, and I am more likely to break something.

[AllenDowney]

But I can clarify the test case. This example:

style='s', color='C3'

Should be legal because the style is specifying a shape, not a color.

Instead it raises "Cannot pass 'style' string with a color symbol and 'color' keyword argument.

The line of code I'm changing checks whether the style string contains any letters a-z, seemingly on the assumption that all letters in a style string are color codes. But there are several letters, including s and o, that indicate shapes, not colors.

[WillAyd]

You can look at the tests defined in pandas/tests/plotting. Both test_series.py and test_frame.py should have examples you can copy; the color validation tests may be the best

[WillAyd]

As mentioned above if you could add tests would be great

[TomAugspurger]

Whoops, I've done something horribly wrong to your branch @AllenDowney. Will fix it up.

[TomAugspurger]

Ahh, I see. This was against 0.25.x. All our PRs go through master first.

[AllenDowney]

thank you!

…

On Tue, Nov 5, 2019, at 1:24 PM, Tom Augspurger wrote: ***@***.**** approved this pull request. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#29122?email_source=notifications&email_token=AAOLP3MAOEMKLLHZRBR3G6LQSG27JA5CNFSM4JCWT4T2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCKL6JAQ#pullrequestreview-311944322>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOLP3JRCRT6Z3VB6TLS3C3QSG27JANCNFSM4JCWT4TQ>.

[TomAugspurger]

Thanks Allen!

[joooeey]

The following test is wrong!

def test_style_single_ok(self):
    s = pd.Series([1, 2])
    ax = s.plot(style="s", color="C3")
    assert ax.lines[0].get_color() == ["C3"]

The color of a single Line2D cannot usually be a list. Perhaps a list of the 3 RGB or the 4 RGBa values. But not ["C3"]. I'll change this test in PR #33821 .