Opened 13 years ago

Closed 13 years ago

Last modified 6 years ago

#8131 closed defect (fixed)

Catalan numbers are integers

Reported by: Francis Clarke Owned by: Sage Combinat CC user
Priority: minor Milestone: sage-4.3.4
Component: combinatorics Keywords: catalan
Cc: Merged in: sage-4.3.4.alpha0
Authors: Francis Clarke Reviewers: Nicolas Borie
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

The patch makes Sage aware of this fact.

Attachments (2)

trac_8131.patch (601 bytes) - added by Francis Clarke 13 years ago.
trac_8131-replacement.patch (1.9 KB) - added by Francis Clarke 13 years ago.
replaces earlier patch, based on 4.3.1

Download all attachments as: .zip

Change History (9)

Changed 13 years ago by Francis Clarke

Attachment: trac_8131.patch added

comment:1 Changed 13 years ago by Francis Clarke

Status: newneeds_review

comment:2 Changed 13 years ago by Nicolas M. Thiéry

Reviewers: Nicolas M. Thiéry
Status: needs_reviewneeds_work

Thanks for spotting and fixing this!

Please add an:

OUTPUT: integer

line in the documentation, as per http://www.sagemath.org/doc/developer/conventions.html#documentation-strings, as well an example in the doctests showing/testing that property, and I'll set a positive review!

Note: you may want to use the method divide_knowing_divisible_by on ZZ elements.

comment:3 in reply to:  2 Changed 13 years ago by Francis Clarke

Status: needs_workneeds_review

Replying to nthiery:

In the replacement patch I've improved the documentation and added a doctest as suggested.

In addition, I've simplified an existing doctest and slightly modified another (because divide_knowing_divisible_by gives a different error when trying to divide by zero). I've made one more adjustment to the documentation so that the mathematics displays properly; at least it does in the notebook now.

Changed 13 years ago by Francis Clarke

Attachment: trac_8131-replacement.patch added

replaces earlier patch, based on 4.3.1

comment:4 Changed 13 years ago by nborie

Reviewers: Nicolas M. ThiéryNicolas Borie
Status: needs_reviewpositive_review

It's look good for code, test and applying. I give this patch a positive review.

comment:5 Changed 13 years ago by Minh Van Nguyen

Merged in: sage-4.3.4.alpha0
Resolution: fixed
Status: positive_reviewclosed

Merged trac_8131-replacement.patch.

Francis: You should put the ticket number in your commit message.

comment:6 Changed 6 years ago by Peleg Michaeli

Keywords: catalan added; Catalan numbers removed

comment:7 in reply to:  6 Changed 6 years ago by Francis Clarke

Replying to pelegm:

The trouble with using the keyword 'catalan' is that it makes it look as though it concerns the Catalan constant rather than the Catalan numbers:

sage: catalan.n()
0.915965594177219
sage: catalan_number(7)
429
Note: See TracTickets for help on using tickets.