Opened 12 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 nthiery)

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 nthiery 6 years ago.
Minor doc tweak

Download all attachments as: .zip

Change History (24)

comment:1 Changed 12 years ago by mabshoff

  • Milestone changed from sage-2.10.3 to sage-2.10.2

comment:2 Changed 11 years ago by mabshoff

Mike,

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

Cheers,

Michael

comment:3 Changed 11 years ago by nthiery

  • Cc sage-combinat added

comment:4 Changed 11 years ago by nthiery

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 7 years ago by tscrim

  • Authors set to Travis Scrimshaw
  • 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 7 years ago by tscrim

  • Dependencies changed from #14085 to #13605 #14085 #14248

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

comment:7 Changed 7 years ago by tscrim

Cleaned up some docstrings.

comment:8 Changed 7 years ago by nthiery

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 tscrim

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 tscrim

  • 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 tscrim

  • Description modified (diff)
  • Summary changed from dynkin diagram weights to dynkin diagram latex

comment:12 Changed 6 years ago by nthiery

  • Description modified (diff)

comment:13 Changed 6 years ago by nthiery

  • 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 6 years ago by nthiery (previous) (diff)

comment:14 Changed 6 years ago by tscrim

  • Status changed from needs_review to positive_review

comment:15 Changed 6 years ago by jdemeyer

  • 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 nthiery

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 nthiery

  • Status changed from needs_work to needs_review

comment:19 Changed 6 years ago by nthiery

  • Status changed from needs_review to positive_review
  • Type changed from defect to enhancement

comment:20 Changed 6 years ago by nthiery

  • 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 nthiery

  • 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 nthiery

  • Summary changed from dynkin diagram latex to Latex pictures for Dynkin diagram and misc improvements to Cartan types

Changed 6 years ago by nthiery

Minor doc tweak

comment:23 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.10.beta3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.