Skip to content

FIX: avoid applying dashed patterns to zero-width lines and patches#31307

Merged
ksunden merged 4 commits intomatplotlib:mainfrom
francisayyad03:main
Mar 20, 2026
Merged

FIX: avoid applying dashed patterns to zero-width lines and patches#31307
ksunden merged 4 commits intomatplotlib:mainfrom
francisayyad03:main

Conversation

@francisayyad03
Copy link
Copy Markdown
Contributor

PR summary

closes #31302

This fixes a draw-time failure for zero-width dashed strokes.

Minimum reproduction:

import matplotlib
matplotlib.use("Agg")
import matplotlib.pyplot as plt

plt.style.use("mpl20")
fig, ax = plt.subplots()
ax.stairs([1, 2, 3, 4], [1, 2, 3, 4, 5], fill=True, ls="--")
fig.canvas.draw()

Before this change, this produced lw=0.0 and _dash_pattern=(0.0, [0.0, 0.0]), then raised ValueError: At least one value in the dash list must be positive.

This patch fixes the issue at draw time by avoiding application of dash patterns when the effective linewidth is zero. That keeps the public artist state unchanged while preventing invalid all-zero dash patterns from being passed to
the graphics context. In particular, this avoids changing stored linestyle state or getter behavior, which keeps the fix narrowly scoped to rendering.

Regression tests are added for zero-width dashed Line2D, filled stairs under mpl20, and zero-width dashed Patch.

AI Disclosure

I used AI as a debugging aid while investigating the failure and drafting candidate regression tests. I reviewed the resulting changes, reproduced the failure locally before the fix, and ran the added tests locally after the fix.

PR checklist

@github-actions
Copy link
Copy Markdown

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide.
Please let us know if (and how) you use AI, it will help us give you better feedback on your PR.

We strive to be a welcoming and open project. Please follow our Code of Conduct.

Copy link
Copy Markdown
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

I think coping with this at draw time is the right approach.

Tests could be a bit more expressive. They are currently only smoke tests, i.e. ensure they don't blow up, but don't test the result.

You may be able to use https://matplotlib.org/stable/api/testing_api.html#matplotlib.testing.decorators.check_figures_equal.
For example in the stairs case, you could draw once with ls='-' and once with ls=--`. The claim would be that both are identical because for a zero linewidth, the linestyle does not matter.

@francisayyad03
Copy link
Copy Markdown
Contributor Author

I updated the regressions to use check_figures_equal, so they now compare the
rendered output of zero width dashed and solid strokes instead of only checking
that draw does not raise.

@QuLogic
Copy link
Copy Markdown
Member

QuLogic commented Mar 16, 2026

This is what I did to get through testing the other PR (albeit in a simpler form), so I do think it's the right approach.

However, if I revert the lines.py/patches.py changes, then only lib/matplotlib/tests/test_axes.py::test_stairs_fill_zero_linewidth fails. So I'm not sure the other tests are adequately checking what you intend.

@QuLogic
Copy link
Copy Markdown
Member

QuLogic commented Mar 16, 2026

I believe you need the mpl20 context on all the new tests, as line scaling is not enabled for the classic style.

@francisayyad03
Copy link
Copy Markdown
Contributor Author

I updated the tests accordingly.
The stairs case still uses check_figures_equal. For the broader Line2D and Patch cases, I switched the tests to run under mpl20 and check the GraphicsContextBase.set_dashes call during a real draw, so they directly
cover the draw-time guard.

Comment thread lib/matplotlib/tests/test_lines.py Outdated
autospec=True,
wraps=matplotlib.backend_bases.GraphicsContextBase.set_dashes,
) as set_dashes:
line.draw(renderer)
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.

You should use fig.draw_without_rendering() instead of reaching in to the internals.

@francisayyad03
Copy link
Copy Markdown
Contributor Author

francisayyad03 commented Mar 17, 2026

I simplified the generic Line2D / Patch regressions to use fig.draw_without_rendering() under mpl20, rather than patching backend internals. The stairs regression still uses check_figures_equal, while the
broader tests remain direct draw regressions for the zero width dashed paths.

@melissawm melissawm moved this from Waiting for author to Needs review in First Time Contributors Mar 17, 2026
Copy link
Copy Markdown
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

Note, I only meant you could avoid the internal renderer stuff, not the mocking. The tests may be more of a smoke test now.

This seems good enough for me, but @timhoffm may still prefer a less smoke-test version.

@ksunden ksunden added this to the v3.11.0 milestone Mar 20, 2026
@ksunden ksunden merged commit 4eed106 into matplotlib:main Mar 20, 2026
36 of 40 checks passed
@github-project-automation github-project-automation Bot moved this from Needs review to Merged in First Time Contributors Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

stairs with dashed linestyle and fill=True raises ValueError

5 participants