Opened 6 years ago

Closed 6 years ago

#14063 closed enhancement (fixed)

Remove CombinatorialClass from Compositions

Reported by: tscrim Owned by: sage-combinat
Priority: major Milestone: sage-5.8
Component: combinatorics Keywords: deprecation
Cc: sage-combinat, nthiery Merged in: sage-5.8.beta2
Authors: Travis Scrimshaw Reviewers: Vincent Delecroix
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #14065 Stopgaps:

Description

Part of #12913.

Attachments (1)

trac_14063-remove_cc_compositions-ts.patch​ (55.3 KB) - added by tscrim 6 years ago.
Fixed docstrings

Download all attachments as: .zip

Change History (15)

comment:1 Changed 6 years ago by tscrim

  • Authors set to Travis Scrimshaw
  • Cc nthiery added
  • Dependencies set to #14065
  • Status changed from new to needs_review

comment:2 Changed 6 years ago by vdelecroix

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

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 6 years ago by tscrim

Made fixes based on your comments.

Thanks,
Travis

comment:5 Changed 6 years ago by vdelecroix

  • Status changed from needs_review to positive_review

Great job !

Vincent

comment:6 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.7 to sage-5.8
  • Reviewers set to Vincent Delecroix

comment:7 Changed 6 years ago by jdemeyer

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

comment:8 Changed 6 years ago by tscrim

Forgot to export, sorry about that. Fixed.

comment:9 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.8.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:10 Changed 6 years ago by jdemeyer

  • Merged in sage-5.8.beta1 deleted
  • Resolution fixed deleted
  • Status changed from closed to new

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 6 years ago by tscrim

  • Status changed from new to needs_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 6 years ago by tscrim

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

Sorry about this,
Travis

Changed 6 years ago by tscrim

Fixed docstrings

comment:13 Changed 6 years ago by tscrim

  • Status changed from needs_review to positive_review

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

comment:14 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.8.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.