Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 8, 2017

Conversation

kenmaca
Copy link
Contributor

@kenmaca 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 QuLogic added this to the 2.0.1 (next bug fix release) milestone Mar 5, 2017
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
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?

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
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.

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


Copy link
Member

Choose a reason for hiding this comment

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

Why remove?

@@ -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()],
Copy link
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
Copy link
Contributor Author

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.

@dkua
Copy link
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.

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
Copy link
Contributor Author

kenmaca commented Mar 7, 2017

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
Copy link
Member

QuLogic commented Mar 7, 2017

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
Copy link
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
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