Opened 7 years ago
Closed 7 years ago
#18184 closed defect (fixed)
CombinatorialObject constructor should copy input
Reported by:  jdemeyer  Owned by:  

Priority:  major  Milestone:  sage6.6 
Component:  combinatorics  Keywords:  
Cc:  Merged in:  
Authors:  Jeroen Demeyer  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  e03140a (Commits, GitHub, GitLab)  Commit:  e03140a761586bf64f01aceba324abd1eece813b 
Dependencies:  Stopgaps: 
Description
Constructing a Partition
(or more generally, a CombinatorialObject
) from a list does not copy this list, leading to:
sage: L = IntegerListsLex(max_slope=1, element_class=Partition) sage: x = [3,2,1] sage: P = L(x) sage: x[0] = 5 sage: list(P) [5, 2, 1]
In the IntegerListsLex
implementation in #17979, a workaround was added for this. Instead, fix this by copying the input.
Change History (6)
comment:1 Changed 7 years ago by
comment:2 Changed 7 years ago by
 Branch set to u/jdemeyer/combinatorialobject_constructor_should_copy_input
comment:3 Changed 7 years ago by
 Commit set to e03140a761586bf64f01aceba324abd1eece813b
 Status changed from new to needs_review
New commits:
e03140a  CombinatorialObject constructor should copy input

comment:4 Changed 7 years ago by
 Reviewers set to Vincent Delecroix
 Status changed from needs_review to positive_review
Looks good (and all tests pass).
Vincent
comment:5 Changed 7 years ago by
Maybe someday we should do something about the name, by the way.
CombinatorialObject provides a thin wrapper around a list
Next to everything I work with is a combinatorial object, and very few of them are lists.
Nathann
comment:6 Changed 7 years ago by
 Branch changed from u/jdemeyer/combinatorialobject_constructor_should_copy_input to e03140a761586bf64f01aceba324abd1eece813b
 Resolution set to fixed
 Status changed from positive_review to closed
Note: See
TracTickets for help on using
tickets.
What about a keyword
copy
in the constructor (which would beTrue
by default)? You can save a lot of time by not copying the list. This is what we did insage.combinat.designs.incidence_structures.IncidenceStructure
.Vincent