Opened 13 years ago
Closed 13 years ago
#5600 closed enhancement (fixed)
[with patch, positive review] Cleanup of integer compositions
Reported by: | nthiery | Owned by: | nthiery |
---|---|---|---|
Priority: | major | Milestone: | sage-4.1.2 |
Component: | combinatorics | Keywords: | integer compositions |
Cc: | sage-combinat, jbandlow | Merged in: | Sage 4.1.2.alpha0 |
Authors: | Nicolas M. Thiéry | Reviewers: | Dan Drake, Jason Bandlow, Minh Van Nguyen |
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
- Documentation improvements
- Fixes some of http://wiki.sagemath.org/combinat/Weirdness
- Composition(l) accepts any iterable l, and in particular a tuple
- New functionalities:
- concatenation (as add and sum)
- size
- fatter, finer, fatten (refinement of compositions)
- Uses IntegerListsLex? (fast iteration, ...) instead of not any better specific code Note: this changes the iteration order to inverse lexicographic, and iteration changes the iteration order for set partitions, skew partitions, and skew tableaux.
Attachments (4)
Change History (16)
comment:1 Changed 13 years ago by
- Description modified (diff)
comment:2 Changed 13 years ago by
- Description modified (diff)
comment:3 Changed 13 years ago by
- Cc sage-combinat-devel@… removed
- Description modified (diff)
comment:4 Changed 13 years ago by
- Cc sage-combinat added
comment:5 Changed 13 years ago by
- Description modified (diff)
- Keywords integer compositions added
- Summary changed from Cleanup of integer compositions to [with patch, needs review] Cleanup of integer compositions
Changed 13 years ago by
comment:6 Changed 13 years ago by
- Status changed from new to assigned
comment:7 Changed 13 years ago by
- Summary changed from [with patch, needs review] Cleanup of integer compositions to [with patch, needs work] Cleanup of integer compositions
comment:8 Changed 13 years ago by
- Cc jbandlow added
- Milestone changed from sage-combinat to sage-4.1.1
- Reviewers set to Dan Drake, Jason Bandlow
- Summary changed from [with patch, needs work] Cleanup of integer compositions to [with patch, needs review] Cleanup of integer compositions
The updated patch fixes the (the failure was actually actually a trivial thing in the doctest; thanks to Jason for spotting this!)
Changed 13 years ago by
comment:9 Changed 13 years ago by
- Summary changed from [with patch, needs review] Cleanup of integer compositions to [with patch, positive review] Cleanup of integer compositions
Doctests pass, and it looks like Dan's issues have been resolved. Positive review from me.
comment:10 follow-up: ↓ 11 Changed 13 years ago by
I'm attaching the reviewer patch trac_5600-reviewer.patch
. It fixes a number of typos introduced by trac_5600-compositions_cleanup-nt.patch
, and edits some docstrings so they look a bit nicer when one views the corresponding pages in the reference manual. If people are happy with my reviewer patch trac_5600-reviewer.patch
, then patches should be merged in this order:
trac_5600-compositions_cleanup-nt.patch
trac_5600-reviewer.patch
comment:11 in reply to: ↑ 10 Changed 13 years ago by
Replying to mvngu:
I'm attaching the reviewer patch
trac_5600-reviewer.patch
. It fixes a number of typos introduced bytrac_5600-compositions_cleanup-nt.patch
, and edits some docstrings so they look a bit nicer when one views the corresponding pages in the reference manual. If people are happy with my reviewer patchtrac_5600-reviewer.patch
, then patches should be merged in this order:
trac_5600-compositions_cleanup-nt.patch
trac_5600-reviewer.patch
Thanks much for fixing all those typos! positive review on your reviewer's patch.
comment:12 Changed 13 years ago by
- Merged in set to Sage 4.1.2.alpha0
- Resolution set to fixed
- Reviewers changed from Dan Drake, Jason Bandlow to Dan Drake, Jason Bandlow, Minh Van Nguyen
- Status changed from assigned to closed
Merged patches in this order:
trac_5600-compositions_cleanup-nt.patch
trac_5600-reviewer.patch
I very much want this patch to get in, since I almost opened a ticket for one of the problems that it fixes. (
Composition()
doesn't take tuples as input.) One thing that needs to change, though, is with the docstring for Composition: right now, it tells the user to doComposition_class?
, but that doesn't work sinceComposition_class
is not in the default namespace. I thinkComposition?
should simply display the correct docstring; there's no need to annoy the user by sending him or her bouncing from one docstring to the next.Another issue is that the AUTHORS block is deleted from the head of the file. This should not be done. In fact, you should add yourself to it as the developer's guide suggests: something like "Nicolas M. Thiery (2009-05-25): several cleanups, new functions, and improvements (trac #5600)". I like listing ticket numbers because interested developers can go see exactly what you changed.
Tiny grammar/spelling issues:
Finally, I am seeing a doctest failure with this patch applied to vanilla 4.0.rc0: