Opened 8 years ago
Closed 8 years ago
#14655 closed defect (fixed)
Dynkin diagrams does not copy like digraph
Reported by: | tscrim | Owned by: | sage-combinat |
---|---|---|---|
Priority: | major | Milestone: | sage-5.11 |
Component: | combinatorics | Keywords: | dynkin diagram copy |
Cc: | sage-combinat, nthiery, mshimo, aschilling | Merged in: | sage-5.11.beta1 |
Authors: | Travis Scrimshaw | Reviewers: | Mark Shimozono |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Initializing:
sage: d = DynkinDiagram(['B',3,1]) sage: g = DiGraph() sage: for x in d.vertices(): g.add_vertex(x) sage: for y in d.edges(): g.add_edge(y) sage: cd = copy(d) sage: cg = copy(g) sage: cd is d False sage: cd == d and cg == g True
Now the correct behavior using DiGraph
:
sage: g.vertices() [0, 1, 2, 3] sage: cg.vertices() [0, 1, 2, 3] sage: cg.add_vertex(4) sage: g.vertices() [0, 1, 2, 3] sage: cg.vertices() [0, 1, 2, 3, 4]
The error with DynkinDiagram
:
sage: d.vertices() [0, 1, 2, 3] sage: cd.vertices() [0, 1, 2, 3] sage: cd.add_vertex(4) sage: cd.vertices() [0, 1, 2, 3, 4] sage: d.vertices() [0, 1, 2, 3, 4]
This causes problems with automorphism_group(edge_labels=True)
noticed by Mark Shimozono.
Attachments (1)
Change History (11)
comment:1 Changed 8 years ago by
- Cc aschilling added
comment:2 Changed 8 years ago by
- Status changed from new to needs_review
comment:3 Changed 8 years ago by
- Reviewers set to mshimo
- Status changed from needs_review to needs_work
Thanks, Travis, for fixing this. A few small things to deal with:
Please fix the docstring for :func:DynkinDiagram
: the real issue for the Cartan matrix convention
is transposing or not; in everyone's conventions the arrow points from a *longer* root to a *shorter* one.
I ran the following tests and got some errors.
sage: d = DynkinDiagram(['A',3,1]) sage: TestSuite(d).run() Failure in _test_not_implemented_methods: Traceback (click to the left of this block for traceback) ... The following tests failed: _test_not_implemented_methods
I'm not sure I understand why, but :class:DynkinDiagram_class
inherits from
:class:CartanType_abstract
and therefore is obligated to supply its abstract methods.
It appears not to fulfill its promise for :meth:is_finite
, :meth:is_affine
, and :meth:is_irreducible
.
Otherwise it looks great!
comment:4 Changed 8 years ago by
- Reviewers changed from mshimo to Mark Shimozono
- Status changed from needs_work to needs_review
I believe it is so that you can easily pass Dynkin diagrams as Cartan types (without using the coercion system). Anyways, new version of the patch which implements the additional methods and makes the docstring fix.
comment:5 Changed 8 years ago by
- Status changed from needs_review to positive_review
comment:6 Changed 8 years ago by
- Milestone changed from sage-5.10 to sage-5.11
comment:7 Changed 8 years ago by
- Status changed from positive_review to needs_info
The patch file is missing some metadata, in particular a username. Did you create the patch using hg export
?
Changed 8 years ago by
comment:8 Changed 8 years ago by
- Status changed from needs_info to needs_review
Forgot to export. Fixed.
comment:9 Changed 8 years ago by
- Status changed from needs_review to positive_review
comment:10 Changed 8 years ago by
- Merged in set to sage-5.11.beta1
- Resolution set to fixed
- Status changed from positive_review to closed
Fixed by making
DynkinDiagram_class.__init__()
handle the input fromGenericGraph.__copy__()
and do some type-checking.