Opened 6 years ago
Closed 6 years ago
#18184 closed defect (fixed)
CombinatorialObject constructor should copy input
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.6 |
Component: | combinatorics | Keywords: | |
Cc: | Merged in: | ||
Authors: | Jeroen Demeyer | Reviewers: | Vincent Delecroix |
Report Upstream: | N/A | Work issues: | |
Branch: | e03140a (Commits) | 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 work-around was added for this. Instead, fix this by copying the input.
Change History (6)
comment:1 Changed 6 years ago by
comment:2 Changed 6 years ago by
- Branch set to u/jdemeyer/combinatorialobject_constructor_should_copy_input
comment:3 Changed 6 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 6 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 6 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 6 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