Issue #8141: Dash validator allowing None values in addition to floats #8196

Merged
merged 1 commit into from Mar 8, 2017

Conversation

Projects
None yet
5 participants
Contributor

kenmaca commented Mar 5, 2017

Validators for dashed linestyles now allow None as an allowed value along with floats if optional allow_none kwarg flag in validate_nseq_float.

Other validators that use validate_nseq_float aren't affected, so this bugfix won't have any breaking changes.

QuLogic added this to the 2.0.1 (next bug fix release) milestone Mar 5, 2017

lib/matplotlib/rcsetup.py
- def __init__(self, n=None):
- self.n = n
+ def __init__(self, n=None, allow_none=False):
+ self.n, self.allow_none = n, allow_none
@QuLogic

QuLogic Mar 5, 2017

Member

Is it very common here to put this on one line?

lib/matplotlib/rcsetup.py
- return [float(val) for val in s]
+ return [float(val)
+ if self.allow_none and val is not None
+ or not self.allow_none
@QuLogic

QuLogic Mar 5, 2017

Member

I think this can be simplified to not self.allow_none or val is not None.

lib/matplotlib/rcsetup.py
@@ -929,7 +933,6 @@ def _validate_linestyle(ls):
raise ValueError("linestyle must be a string or " +
"an even-length sequence of floats.")
-
@QuLogic

QuLogic Mar 5, 2017

Member

Why remove?

lib/matplotlib/rcsetup.py
@@ -964,7 +967,8 @@ def _validate_linestyle(ls):
'lines.dash_capstyle': ['butt', validate_capstyle],
'lines.solid_capstyle': ['projecting', validate_capstyle],
'lines.dashed_pattern': [[3.7, 1.6], validate_nseq_float()],
- 'lines.dashdot_pattern': [[6.4, 1.6, 1, 1.6], validate_nseq_float()],
+ 'lines.dashdot_pattern': [[6.4, 1.6, 1, 1.6],
+ validate_nseq_float()],
@QuLogic

QuLogic Mar 5, 2017

Member

Why change this?

Contributor

kenmaca commented Mar 5, 2017

Thanks @QuLogic. I apologize for the novice mistakes and have made appropriate corrections. This seems to fix the initial problem, but it seems CI is failing -- will investigate further.

Contributor

dkua commented Mar 5, 2017

It would probably be good to include a test for the regression this PR is fixing. There's a good place in https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/tests/test_cycles.py to add in a test. You might also want to think about rebasing the "Fixed" and "Changed" into one commit as it's better PR style.

I'm assuming you're doing this for CSCD01? If you are, tell Anya I said hi.

@kenmaca kenmaca Fixes #8141
Validators for dashed linestyles now allow None as an allowed value along with floats through optional `allow_none` kwarg in validate_nseq_float. Other validators that use validate_nseq_float arent affected
be72573
Contributor

kenmaca commented Mar 7, 2017 edited

I've added a test and cleaned up my commits. Looks like AppVeyor decided to play nice this time -- couldn't figure out why it was failing the single build (likely something unrelated).

Also, looks like I forgot to reference the original issue that this PR fixes: #8141

Member

QuLogic commented Mar 7, 2017

Looks okay, I think, but can you amend the commit to give the correct first line.

@NelleV

NelleV approved these changes Mar 8, 2017

@NelleV NelleV merged commit f7341b9 into matplotlib:master Mar 8, 2017

5 checks passed

codecov/patch 100% of diff hit (target 80%)
Details
codecov/project/library 57.99% (+0.07%) compared to 1f173dd
Details
codecov/project/tests 98.56% (target 97.9%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@QuLogic QuLogic added a commit to QuLogic/matplotlib that referenced this pull request Mar 27, 2017

@NelleV @QuLogic NelleV + QuLogic Merge pull request #8196 from kenmaca/pr-dash-verification-8141
Issue #8141: Dash validator allowing None values in addition to floats
058f6b1

QuLogic removed the Needs backport label Mar 27, 2017

Member

QuLogic commented Mar 27, 2017

Backported to v2.0.x as 058f6b1.

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