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 mantepse)

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 mantepse

  • Authors set to Martin Rubey
  • Component changed from PLEASE CHANGE to combinatorics
  • Description modified (diff)
  • Type changed from PLEASE CHANGE to enhancement

comment:2 Changed 19 months ago by mantepse

  • Branch set to u/mantepse/move_methods_on_general_set_partitions_into_abstractsetpartition

comment:3 Changed 19 months ago by tscrim

  • Cc sage-combinat tscrim darij alauve zabrocki added
  • Commit set to 29518ce393c6fc6ef66b136edc7c5bbc76da9f04

New commits:

29518cemove methods into AbstractSetPartition class

comment:4 follow-up: Changed 19 months ago by zabrocki

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: Changed 19 months ago by mantepse

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 Sets. 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 zabrocki

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

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.

Note: See TracTickets for help on using tickets.