Opened 10 years ago

Closed 10 years ago

#14063 closed enhancement (fixed)

Remove CombinatorialClass from Compositions

Reported by: Travis Scrimshaw Owned by: Sage Combinat CC user
Priority: major Milestone: sage-5.8
Component: combinatorics Keywords: deprecation
Cc: Sage Combinat CC user, Nicolas M. Thiéry Merged in: sage-5.8.beta2
Authors: Travis Scrimshaw Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #14065 Stopgaps:

Status badges

Description

Part of #12913.

Attachments (1)

trac_14063-remove_cc_compositions-ts.patch (55.3 KB) - added by Travis Scrimshaw 10 years ago.
Fixed docstrings

Download all attachments as: .zip

Change History (15)

comment:1 Changed 10 years ago by Travis Scrimshaw

Authors: Travis Scrimshaw
Cc: Nicolas M. Thiéry added
Dependencies: #14065
Status: newneeds_review

comment:2 Changed 10 years ago by Vincent Delecroix

Hi,

  • composition_from_subset must be renamed to from_subset
  • You may remove the few trailing whitespaces that remain.

The rest looks good.

Vincent

comment:3 Changed 10 years ago by Vincent Delecroix

Comments on the doc in composition.py

  • line 786: list(Compositions(4)) may be replaced with Compositions(4).list() (idem lines 877, 882, 927)
  • line 844: there is a strange sharp at the end of the line
  • line 859: a space is missing

comment:4 Changed 10 years ago by Travis Scrimshaw

Made fixes based on your comments.

Thanks,
Travis

comment:5 Changed 10 years ago by Vincent Delecroix

Status: needs_reviewpositive_review

Great job !

Vincent

comment:6 Changed 10 years ago by Jeroen Demeyer

Milestone: sage-5.7sage-5.8
Reviewers: Vincent Delecroix

comment:7 Changed 10 years ago by Jeroen Demeyer

How was this patch file created, as some headers are missing? You should use hg export [tip].

comment:8 Changed 10 years ago by Travis Scrimshaw

Forgot to export, sorry about that. Fixed.

comment:9 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.8.beta1
Resolution: fixed
Status: positive_reviewclosed

comment:10 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.8.beta1
Resolution: fixed
Status: closednew

The LaTeX documentation doesn't build because you use backslashes inside the

def cardinality(self):
"""
"""

docstring instead of

def cardinality(self):
r"""
"""

comment:11 Changed 10 years ago by Travis Scrimshaw

Status: newneeds_review

Made the fixes (I put it in a few other places as well). I'll double/triple-check over the compiled docbuild once it finishes the rebuild.

comment:12 Changed 10 years ago by Travis Scrimshaw

I also fixed a broken link. Jeroen, would it be okay if I set this back to positive review?

Sorry about this,
Travis

Changed 10 years ago by Travis Scrimshaw

Fixed docstrings

comment:13 Changed 10 years ago by Travis Scrimshaw

Status: needs_reviewpositive_review

I'm setting this back to positive review...I hope that's okay...

comment:14 Changed 10 years ago by Jeroen Demeyer

Merged in: sage-5.8.beta2
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.