Skip to content

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

Merged
NelleV merged 1 commit into
matplotlib:masterfrom
kenmaca:pr-dash-verification-8141
Mar 8, 2017
Merged

NelleV merged 1 commit into
matplotlib:masterfrom
kenmaca:pr-dash-verification-8141

Conversation

@kenmaca

@kenmaca kenmaca commented Mar 5, 2017

Copy link
Copy Markdown
Contributor

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 QuLogic added this to the 2.0.1 (next bug fix release) milestone Mar 5, 2017
Comment thread lib/matplotlib/rcsetup.py Outdated
def __init__(self, n=None):
self.n = n
def __init__(self, n=None, allow_none=False):
self.n, self.allow_none = n, allow_none

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread lib/matplotlib/rcsetup.py Outdated
return [float(val) for val in s]
return [float(val)
if self.allow_none and val is not None
or not self.allow_none

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread lib/matplotlib/rcsetup.py
raise ValueError("linestyle must be a string or " +
"an even-length sequence of floats.")


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove?

Comment thread lib/matplotlib/rcsetup.py Outdated
'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()],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this?

@kenmaca

kenmaca commented Mar 5, 2017

Copy link
Copy Markdown
Contributor Author

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.

@dkua

dkua commented Mar 5, 2017

Copy link
Copy Markdown
Contributor

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.

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
@kenmaca

kenmaca commented Mar 7, 2017 •

Copy link
Copy Markdown
Contributor Author

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

@QuLogic

QuLogic commented Mar 7, 2017

Copy link
Copy Markdown
Member

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

@NelleV
NelleV merged commit f7341b9 into matplotlib:master Mar 8, 2017
@QuLogic QuLogic added Needs backport Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labels Mar 8, 2017
QuLogic pushed a commit to QuLogic/matplotlib that referenced this pull request Mar 27, 2017
…8141

Issue matplotlib#8141: Dash validator allowing None values in addition to floats
@QuLogic

QuLogic commented Mar 27, 2017

Copy link
Copy Markdown
Member

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

Labels

Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants