-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Cleanup tripcolor() #22356
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
Cleanup tripcolor() #22356
Conversation
382eca7
to
a8b9429
Compare
|
||
collection = PolyCollection(verts, **kwargs) | ||
|
||
collection.set_alpha(alpha) | ||
collection.set_array(C) | ||
collection.set_array(colors) |
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.
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.
But there is collection._scale_norm(norm, vmin, vmax)
after setting the norm here. AFAICS this handles the issue in #21146/#21525. One can probably remove that when delaying kwargs setting. But since this PR is already complicated enough and there is no immediate problem, I'd not go into this within this PR.
be7e1e2
to
ef81739
Compare
This issues more and more precise warnings on usage errors but does not change behavior. Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Oops, thought there was a second review; hope you're okay with this @anntzer? |
No worries, opened #22726 to track the rest. |
PR Summary
Commits:
Triangulation.get_from_args_and_kwargs
to make parameter parsing testable; and add tests.triangles
ormask
alongside aTriangulation
totripcolor()
tripcolor()
This issues additional warnings and more precise error messages but does not change behavior.
The only "semantic" change is that I have discouraged passing triangles positionally:
tripcolor(x, y, triangles, C)
is a bit odd and IMHO has too many positional arguments.
Closes #22303. Follows up on #10148.