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:

Status badges

Description (last modified by Nicolas M. Thiéry)

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)

trac_2023-dynkin_graphs-ts.patch (121.0 KB) - added by Nicolas M. Thiéry 10 years ago.
Minor doc tweak

Download all attachments as: .zip

Change History (24)

comment:1 Changed 15 years ago by Michael Abshoff

Milestone: sage-2.10.3sage-2.10.2

comment:2 Changed 14 years ago by Michael Abshoff

Mike,

this has been sitting around for a while. What is the status here?

Cheers,

Michael

comment:3 Changed 14 years ago by Nicolas M. Thiéry

Cc: Sage Combinat CC user added

comment:4 Changed 14 years ago by Nicolas M. Thiéry

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 Travis Scrimshaw

Authors: Travis Scrimshaw
Dependencies: #14085
Report Upstream: N/A
Status: newneeds_review

I've uploaded a patch that gives custom latex printing for Dynkin diagrams for crystallographic types.

comment:6 Changed 10 years ago by Travis Scrimshaw

Dependencies: #14085#13605 #14085 #14248

New version which uses the global options framework for notation choices.

comment:7 Changed 10 years ago by Travis Scrimshaw

Cleaned up some docstrings.

comment:8 Changed 10 years ago by Nicolas M. Thiéry

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 Travis Scrimshaw

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 Travis Scrimshaw

Reviewers: Nicolas M. Thiéry

Updated with a better note about the conventions used in sage.

comment:11 Changed 10 years ago by Travis Scrimshaw

Description: modified (diff)
Summary: dynkin diagram weightsdynkin diagram latex

comment:12 Changed 10 years ago by Nicolas M. Thiéry

Description: modified (diff)

comment:13 Changed 10 years ago by Nicolas M. Thiéry

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.

Last edited 10 years ago by Nicolas M. Thiéry (previous) (diff)

comment:14 Changed 10 years ago by Travis Scrimshaw

Status: needs_reviewpositive_review

comment:15 Changed 10 years ago by Jeroen Demeyer

Status: positive_reviewneeds_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 Nicolas M. Thiéry

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 Nicolas M. Thiéry

Status: needs_workneeds_review

comment:19 Changed 10 years ago by Nicolas M. Thiéry

Status: needs_reviewpositive_review
Type: defectenhancement

comment:20 Changed 10 years ago by Nicolas M. Thiéry

Status: positive_reviewneeds_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 Nicolas M. Thiéry

Status: needs_workpositive_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 Nicolas M. Thiéry

Summary: dynkin diagram latexLatex pictures for Dynkin diagram and misc improvements to Cartan types

Changed 10 years ago by Nicolas M. Thiéry

Minor doc tweak

comment:23 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.10.beta3
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.