-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Enh improve agg chunks error #19343
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
Enh improve agg chunks error #19343
Conversation
150617a
to
6cfa6ed
Compare
c01a41d
to
52f7610
Compare
52f7610
to
c19d49f
Compare
c19d49f
to
51ae0a8
Compare
The error messages now provide better instruction on how to fix the problem. There are some paths we can not split (one with hatching, fills, or that claim they can not be simplified) so tell user that. If we could have otherwise split up the path if the chunksize is set to less than 100, which disables the feature, ask user to make it bigger or make the path simplification more aggressive. If the chunksize is bigger than the data, ask user to make it smaller or make the path simplification more aggressive. If the chunksize is smaller than the data, but still too big ask user to make it smaller or make the path simplification more aggressive. closes matplotlib#19325 Co-authored-by: Antony Lee <anntzer.lee@gmail.com>
This allows longer, more textured, paths to be drawn with the default settings at the risk of very long run times.
- Make sure values are be big enough / small enough - make long path test more pathological (Random data will sometimes go in the same direction)
51ae0a8
to
09fa321
Compare
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've had a look over and it looks good - there's one change I don't understand at all though (left a comment) so I'm not personally comfortable approving yet.
@@ -45,7 +45,7 @@ RendererAgg::RendererAgg(unsigned int width, unsigned int height, double dpi) | |||
rendererBase(), | |||
rendererAA(), | |||
rendererBin(), | |||
theRasterizer(8192), | |||
theRasterizer(32768), |
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.
What's the meaning of this change?
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.
And does it need documenting in an API change note?
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.
That is:
- makes the upper limit higher so it will be hit less often
…343-on-v3.5.x Backport PR #19343 on branch v3.5.x (Enh improve agg chunks error)
…unks_error Enh improve agg chunks error
Enh improve agg chunks error
PR Summary
This does 2 important things:
and 2 less important things:
PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
and runflake8 --docstring-convention=all
).