Opened 19 months ago
Last modified 19 months ago
#25451 new enhancement
move methods on general set partitions into AbstractSetPartition
Reported by: | mantepse | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.3 |
Component: | combinatorics | Keywords: | |
Cc: | sage-combinat, tscrim, darij, alauve, zabrocki | Merged in: | |
Authors: | Martin Rubey | Reviewers: | |
Report Upstream: | N/A | Work issues: | |
Branch: | u/mantepse/move_methods_on_general_set_partitions_into_abstractsetpartition (Commits) | Commit: | 29518ce393c6fc6ef66b136edc7c5bbc76da9f04 |
Dependencies: | Stopgaps: |
Description (last modified by )
Several methods currently in SetPartition
only make sense for set partitions of {1,...,n}.
As a first step towards resolving this, this tickets moves all the other methods into AbstractSetPartition
.
It is not clear to me yet, however, how to proceed beyond that.
Change History (6)
comment:1 Changed 19 months ago by
- Component changed from PLEASE CHANGE to combinatorics
- Description modified (diff)
- Type changed from PLEASE CHANGE to enhancement
comment:2 Changed 19 months ago by
- Branch set to u/mantepse/move_methods_on_general_set_partitions_into_abstractsetpartition
comment:3 Changed 19 months ago by
- Cc sage-combinat tscrim darij alauve zabrocki added
- Commit set to 29518ce393c6fc6ef66b136edc7c5bbc76da9f04
comment:4 follow-up: ↓ 5 Changed 19 months ago by
For every method in AbstractSetPartition
I think that there should be a doc test for elements of PartitionDiagram
(since PartitionDiagram
inherits from AbstractSetPartition
). In particular, strict_coarsenings
raises an error, but I've been meaning to go through each of the methods and add a doc test because I know that there are others and making changes to this class will inadvertently introduce additional bugs.
comment:5 in reply to: ↑ 4 ; follow-up: ↓ 6 Changed 19 months ago by
Replying to zabrocki:
In particular,
strict_coarsenings
raises an error
The problem is simply that this (and other methods) of AbstractSetPartition
expect self
to be a ClonableArray
of Set
s. This is enforced in SetPartition.__init__
.
I think one question is what the purpose of AbstractSetPartition
really is. It now seems to me that it was intended as a class that allows a different representation of set partitions (eg., as a list of tuples, as in PartitionDiagram
).
I think it would make sense to have one class for set partitions of {1,...,n}, and another one for general set partitions. The same goes for perfect matchings, permutations, etc. But maybe that's not really useful.
comment:6 in reply to: ↑ 5 Changed 19 months ago by
I think one question is what the purpose of
AbstractSetPartition
really is. It now seems to me that it was intended as a class that allows a different representation of set partitions (eg., as a list of tuples, as inPartitionDiagram
).
AbstractSetPartition
is supposed to contain methods that are common to SetPartition
and diagrams from diagram algebras (e.g. PartitionDiagrams
, BrauerDiagrams
, etc.) so that we are not copy/pasting code. Before #25146, diagrams were inheriting all the methods from SetPartition
and some of them were not appropriate.
New commits:
move methods into AbstractSetPartition class