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
Add strands with different colors in BraidGroup plot #14533
Comments
Attachment: trac_14533_braid.patch.gz Attachment: colored_braid.png A braid with different colored strands |
Attachment: red_braid.png A braid with different colored red strands |
This comment has been minimized.
This comment has been minimized.
Reviewer: Frédéric Chapoton |
comment:2
looks good to me. If you are happy with my review (add test and removes some unused imports) then you can set a positive review |
comment:3
Attachment: trac_14533_braid-review-fc.patch.gz |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Attachment: trac_14533_braid2.patch.gz |
This comment has been minimized.
This comment has been minimized.
Author: Tomer Bauer |
comment:6
Replying to @fchapoton:
Thanks for your review. I had a trivial whitespace nitpick I added in attachment: trac_14533_braid2.patch. I think it's OK now. This is new to me; Who should set the review status now? |
comment:7
well, there is no rule, as far as I know, but maybe not the author of the "latest patch".. Anyway, looks good to me, positive review ! |
Merged: sage-5.10.beta3 |
Currently, when plotting a braid, all the strands are in the same color. For
plot3d()
, even choosing a different color is unavailable.The attached patch adds the ability to give each strand a different color. This is done for
plot()
andplot3d()
.Few notes:
color
is a list or a tuple I used the idiomisinstance(color, (list, tuple))
which is of prevalent use in Sage. Should we useisinstance(color, collections.Sequence)
instead, with the downside that we need to checkisinstance(color, types.StringTypes)
first?Two examples:
Apply:
Component: group theory
Keywords: braid group, plot
Author: Tomer Bauer
Reviewer: Frédéric Chapoton
Merged: sage-5.10.beta3
Issue created by migration from https://trac.sagemath.org/ticket/14533
The text was updated successfully, but these errors were encountered: