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: sage-6.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:

Status badges

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

What about a keyword copy in the constructor (which would be True by default)? You can save a lot of time by not copying the list. This is what we did in sage.combinat.designs.incidence_structures.IncidenceStructure.

Vincent

comment:2 Changed 7 years ago by jdemeyer

  • Branch set to u/jdemeyer/combinatorialobject_constructor_should_copy_input

comment:3 Changed 7 years ago by jdemeyer

  • Commit set to e03140a761586bf64f01aceba324abd1eece813b
  • Status changed from new to needs_review

New commits:

​e03140aCombinatorialObject constructor should copy input

comment:4 Changed 7 years ago by vdelecroix

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

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 vbraun

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