Opened 15 years ago
Closed 10 years ago
#2023 closed enhancement (fixed)
Latex pictures for Dynkin diagram and misc improvements to Cartan types
Reported by: | David Joyner | Owned by: | Mike Hansen |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.10 |
Component: | combinatorics | Keywords: | |
Cc: | Sage Combinat CC user | Merged in: | sage-5.10.beta3 |
Authors: | Travis Scrimshaw | Reviewers: | Nicolas M. Thiéry |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #13605 #14085 #14248 | Stopgaps: |
Description (last modified by )
Displaying nice latex pictures for all Dynkin diagrams of affine and classical types.
Also:
- Improves the description of which conventions are used for Dynkin diagrams and Cartan matrices (Bourbaki/Kac?).
- Misc improvements to Cartan types, including a global option for displaying Cartan types using Kac's conventions and much documentation cleanup.
Attachments (1)
Change History (24)
comment:1 Changed 15 years ago by
Milestone: | sage-2.10.3 → sage-2.10.2 |
---|
comment:2 Changed 14 years ago by
comment:3 Changed 14 years ago by
Cc: | Sage Combinat CC user added |
---|
comment:4 Changed 14 years ago by
It's indeed a bit of an abuse to have Dynkin diagram derive from Digraphs (some edges are not oriented). But they are not Graphs either (some edges are). They don't really deserve a special class graph just for themselves, do they? So, I guess we can live with this abuse.
That being said, plot should definitely be overriden to get appropriate pictures. Volunteers?
See also #5502 for ascii art drawing.
comment:5 Changed 10 years ago by
Authors: | → Travis Scrimshaw |
---|---|
Dependencies: | → #14085 |
Report Upstream: | → N/A |
Status: | new → needs_review |
I've uploaded a patch that gives custom latex printing for Dynkin diagrams for crystallographic types.
comment:6 Changed 10 years ago by
Dependencies: | #14085 → #13605 #14085 #14248 |
---|
New version which uses the global options framework for notation choices.
comment:8 Changed 10 years ago by
Hi Travis,
I pushed a reviewer patch on the queue which makes the logic more concise as we had discussed this morning. Please check my changes and fold them. Due to some changes I undid in my patch, you probably want to have a look at the folded patch, and strip away trivial space changes that could be left due to uncomplete undoes.
I'll then have a final check on the updated patch.
Cheers,
Nicolas
comment:9 Changed 10 years ago by
Hey Nicolas,
Thank you for the review. I had to make some minor tweaks to affine types B,C, and D. However this patch will change depending on what happens in #14248.
Thanks,
Travis
comment:10 Changed 10 years ago by
Reviewers: | → Nicolas M. Thiéry |
---|
Updated with a better note about the conventions used in sage.
comment:11 Changed 10 years ago by
Description: | modified (diff) |
---|---|
Summary: | dynkin diagram weights → dynkin diagram latex |
comment:12 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:13 Changed 10 years ago by
Description: | modified (diff) |
---|
I have just been through the patch, and wrote a little reviewer patch which I just pushed to the Sage-Combinat queue. It sounds good to go assuming all tests pass.
Travis: if you are happy with the reviewer patch, please fold upload and set a positive review on my behalf.
comment:14 Changed 10 years ago by
Status: | needs_review → positive_review |
---|
comment:15 Changed 10 years ago by
Status: | positive_review → needs_work |
---|
dochtml.log:[combinat ] /mazur/release/merger/sage-5.10.beta3/devel/sage/doc/en/reference/combinat/sage/combinat/root_system/cartan_type.rst:11: WARNING: error while formatting signature for sage.combinat.root_system.cartan_type.CartanType_crystalographic.ascii_art: invalid syntax (<unknown>, line 1)
comment:16 Changed 10 years ago by
Sorry, we should have caught that. Worked around in the attached patch. See also #14553.
The updated patch was checked by Travis. I am running the tests now.
comment:17 Changed 10 years ago by
Status: | needs_work → needs_review |
---|
comment:18 Changed 10 years ago by
All test passed on sage.math.u-psud.fr and documentation built smoothly:
For the full logs, see:
comment:19 Changed 10 years ago by
Status: | needs_review → positive_review |
---|---|
Type: | defect → enhancement |
comment:20 Changed 10 years ago by
Status: | positive_review → needs_work |
---|---|
Work issues: | → ignored doctests with the new doctest framework |
Some doctests are ignored with the new doctest framework, and the framework complains about it with long tests.
See discussion on:
https://groups.google.com/forum/?fromgroups=#!topic/sage-devel/4m1ydGdiGf8
comment:21 Changed 10 years ago by
Status: | needs_work → positive_review |
---|---|
Work issues: | ignored doctests with the new doctest framework |
The new patches contains two little changes we agreed with Travis / on sage-devel:
- Some trailing whitespace in new lines
- Updating the number of currently ignored tests in doctest/sources
All long test passed.
Back to positive review!
comment:22 Changed 10 years ago by
Summary: | dynkin diagram latex → Latex pictures for Dynkin diagram and misc improvements to Cartan types |
---|
comment:23 Changed 10 years ago by
Merged in: | → sage-5.10.beta3 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Mike,
this has been sitting around for a while. What is the status here?
Cheers,
Michael