-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Require non-zero dash value #22569
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
Require non-zero dash value #22569
Conversation
Maybe one would also like to validate the rcParams like this? No negative and at least one non-zero? Or simply go with all positive (no zeros)? |
We can of course decide on whatever behavior we want, but note that at least cairo and svg allow zero-length "on" segments, as these will draw squares if the linecap is square and circles if the linecap is round (https://cairographics.org/manual/cairo-cairo-t.html#cairo-set-dash, https://svgwg.org/svg2-draft/paths.html#ZeroLengthSegments). |
OK! Then I think it makes sense to still allow it (even without your insight, it can still break existing scripts if we disallow it). |
11842f7
to
463cd8a
Compare
463cd8a
to
7a279dd
Compare
I think the right check is that the sequence seeds to sum to 0. I suspect the hang reported by that user is because we are trying to draw a line, but only taking 0 sized steps. Having at least 1 non-zero step would fix that. |
So negative steps are OK? If not, I do not really see the difference between checking for at least one positive value and sum != 0. |
At least Cairo does not allow negative segments (https://cairographics.org/manual/cairo-cairo-t.html#cairo-set-dash). I suppose if they don't have it, we don't need it either. |
PR Summary
In #14498 (comment) it is mentioned that an all zero
dashes
hangs.This PR introduces an exception for this case.
One may consider changing
matplotlib/lib/matplotlib/backend_bases.py
Line 925 in c57c878
I did not manage to test this properly. Code that generates exceptions in the shell does not do it in the tests. Not sure why.
PR Checklist
Tests and Styling
pytest
passes).flake8-docstrings
and runflake8 --docstring-convention=all
).Documentation
doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).