NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

Fix issue with sketch not working on PathCollection in Agg#25645

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

QuLogic merged 1 commit intomatplotlib:mainfromoscargus:aggsketchcollection

Sep 26, 2024

Merged

Fix issue with sketch not working on PathCollection in Agg#25645

QuLogic merged 1 commit intomatplotlib:mainfromoscargus:aggsketchcollection

Sep 26, 2024

Conversation

Copy link

Member

oscargus commentedApr 7, 2023•
edited
Loading

PR Summary

Closes#19693

Seems like sketch was not handled at all for path collections.

Maybe not the nicest code, but my C/C++ is not that good.

If I understand it correctly, sketch isn't really tested (as it is hard to recreate the images?)?

Here is the output anyway:

PR Checklist

Documentation and Tests

Has pytest style unit tests (andpytest passes)
Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
New plotting related features are documented with examples.

Release Notes

New features are marked with a.. versionadded:: directive in the docstring and documented indoc/users/next_whats_new/
API changes are marked with a.. versionchanged:: directive in the docstring and documented indoc/api/next_api_changes/
Release notes conform with instructions innext_whats_new/README.rst ornext_api_changes/README.rst

oscargus added the backend: agg label

Apr 7, 2023

oscargus added this to thev3.8.0 milestone

Apr 7, 2023

oscargus force-pushed theaggsketchcollection branch from49b3d47 tob235cbbCompare

April 7, 2023 12:43

Copy link

Member

story645 commentedApr 9, 2023

Can you try an image test? I think they're recreatable if you use a fixed seed?

Copy link

MemberAuthor

oscargus commentedApr 10, 2023

We can only control the seed for data generation in NumPy, not the seed for the C++-part.

However, there is#13479 which is related.

oscargus added the status: needs review label

Apr 17, 2023

Copy link

Member

story645 commentedApr 20, 2023

yeah, I agree with Jody that#13479 (comment) is the correct solution (and also probably out of scope here)

Copy link

MemberAuthor

oscargus commentedApr 27, 2023

I've realized that sketching is in general not tested that well (if at all). However, although the result is (probably) deterministic at the moment, I think that it is better to add tests as part of implementing seeding.