Opened 11 years ago
Closed 6 years ago
#2023 closed enhancement (fixed)
Latex pictures for Dynkin diagram and misc improvements to Cartan types
Reported by: | wdj | Owned by: | mhansen |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.10 |
Component: | combinatorics | Keywords: | |
Cc: | sage-combinat | 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 11 years ago by
- Milestone changed from sage-2.10.3 to sage-2.10.2
comment:2 Changed 11 years ago by
comment:3 Changed 10 years ago by
- Cc sage-combinat added
comment:4 Changed 10 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 6 years ago by
- Dependencies set to #14085
- Report Upstream set to N/A
- Status changed from new to needs_review
I've uploaded a patch that gives custom latex printing for Dynkin diagrams for crystallographic types.
comment:6 Changed 6 years ago by
- Dependencies changed from #14085 to #13605 #14085 #14248
New version which uses the global options framework for notation choices.
comment:7 Changed 6 years ago by
Cleaned up some docstrings.
comment:8 Changed 6 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 6 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 6 years ago by
- Reviewers set to Nicolas M. Thiéry
Updated with a better note about the conventions used in sage.
comment:11 Changed 6 years ago by
- Description modified (diff)
- Summary changed from dynkin diagram weights to dynkin diagram latex
comment:12 Changed 6 years ago by
- Description modified (diff)
comment:13 Changed 6 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 6 years ago by
- Status changed from needs_review to positive_review
comment:15 Changed 6 years ago by
- Status changed from positive_review to 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 6 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 6 years ago by
- Status changed from needs_work to needs_review
comment:18 Changed 6 years ago by
All test passed on sage.math.u-psud.fr and documentation built smoothly:
For the full logs, see:
comment:19 Changed 6 years ago by
- Status changed from needs_review to positive_review
- Type changed from defect to enhancement
comment:20 Changed 6 years ago by
- Status changed from positive_review to needs_work
- Work issues set to 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 6 years ago by
- Status changed from needs_work to positive_review
- Work issues ignored doctests with the new doctest framework deleted
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 6 years ago by
- Summary changed from dynkin diagram latex to Latex pictures for Dynkin diagram and misc improvements to Cartan types
comment:23 Changed 6 years ago by
- Merged in set to sage-5.10.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
Mike,
this has been sitting around for a while. What is the status here?
Cheers,
Michael