-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
Conversation
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 |
There was a problem hiding this comment.
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?
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 |
There was a problem hiding this comment.
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
.
lib/matplotlib/rcsetup.py
Outdated
@@ -929,7 +933,6 @@ def _validate_linestyle(ls): | |||
raise ValueError("linestyle must be a string or " + | |||
"an even-length sequence of floats.") | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove?
lib/matplotlib/rcsetup.py
Outdated
@@ -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()], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change this?
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. |
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
8d33b9b
to
be72573
Compare
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 |
Looks okay, I think, but can you amend the commit to give the correct first line. |
…8141 Issue matplotlib#8141: Dash validator allowing None values in addition to floats
Backported to |
Validators for dashed linestyles now allow
None
as an allowed value along with floats if optionalallow_none
kwarg flag invalidate_nseq_float
.Other validators that use
validate_nseq_float
aren't affected, so this bugfix won't have any breaking changes.