Opened 5 years ago

Closed 5 years ago

#24485 closed enhancement (fixed)

get rid of CombinatorialClass in Necklaces

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.2
Component: combinatorics Keywords:
Cc: tscrim Merged in:
Authors: Frédéric Chapoton Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: cb716c9 (Commits, GitHub, GitLab) Commit: cb716c93d3aab753e4701ccd209a9f58fbe7a728
Dependencies: Stopgaps:

Status badges

Description

as part of #12913

Change History (6)

comment:1 Changed 5 years ago by chapoton

Branch: → u/chapoton/24485
Commit: → fbecb41c541d81ff1bf963fe3deadf8bc125c6e9
Status: new → needs_review

New commits:

​fbecb41refresh the necklaces (no more CombinatorialClass)

comment:2 Changed 5 years ago by tscrim

Reviewers: → Travis Scrimshaw

Two things: __classcall_private__ needs a doctest: 

sage: Necklaces([2,1,1]) is Necklaces(Composition([2,1,1]))
True

I think you should just (attempt to) convert into a Composition when content is not a Composition.

Also, I was told at some point that it is better to have UniqueRepresentation be before Parent, but I don't remember precisely why (and don't feel like you have to change it).

comment:3 Changed 5 years ago by git

Commit: fbecb41c541d81ff1bf963fe3deadf8bc125c6e9 → cb716c93d3aab753e4701ccd209a9f58fbe7a728

Branch pushed to git repo; I updated commit sha1. New commits:

​cb716c9details

comment:4 Changed 5 years ago by chapoton

thanks. done

comment:5 Changed 5 years ago by tscrim

Status: needs_review → positive_review

Thank you.

comment:6 Changed 5 years ago by vbraun

Branch: u/chapoton/24485 → cb716c93d3aab753e4701ccd209a9f58fbe7a728
Resolution: → fixed
Status: positive_review → closed
Note: See TracTickets for help on using tickets.