#13243 closed enhancement (fixed)
new methods for compositions
Reported by: | saliola | Owned by: | sage-combinat |
---|---|---|---|
Priority: | major | Milestone: | sage-5.3 |
Component: | combinatorics | Keywords: | compositions, ncsf, sd40 |
Cc: | chrisjamesberg, sage-combinat, zabrocki | Merged in: | sage-5.3.beta0 |
Authors: | Franco Saliola, Nicolas M. Thiéry, Florent Hivert, Chris Berg | Reviewers: | Chris Berg, Mike Zabrocki |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #13109 | Stopgaps: |
Description (last modified by )
new functionality for the composition constructor:
- construct a composition from a subset
new functionality for compositions:
- parent -- returns Compositions()
- reversed - reverses
- refinement_splitting
- refinement_splitting_lengths
- deprecated refinement in favour of refinement_splitting_lengths
- partial_sums(self, final=True):
- to_subset(self, final=False):
- to_partition(self):
- shuffle_product(self, other, overlap=False):
new functionality for the set of all compositions:
- put it in the category of
InfiniteEnumeratedSets
- subset method to extract subsets of composition (by size)
Apply:
Attachments (6)
Change History (34)
comment:1 Changed 9 years ago by
- Status changed from new to needs_review
comment:2 Changed 9 years ago by
- Status changed from needs_review to positive_review
comment:3 Changed 9 years ago by
- Status changed from positive_review to needs_work
comment:4 Changed 9 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
The errors were mainly due to the fact that the new category of graded enumerated sets does not yet exist (it exists on the sage-combinat queue, which is why tests passed on my machine). So I removed the two lines related to that.
I also modified a couple of doctests in permutation.py
, which was caused by the change the descents method.
comment:5 Changed 9 years ago by
Apply: trac_13243-new_methods_for_compositions-fs.patch
comment:6 Changed 9 years ago by
- Cc zabrocki added
comment:7 Changed 9 years ago by
- Dependencies set to #13109
Apply: trac_13243-new_methods_for_compositions-fs.patch, trac_13243-rebased_deprecated_function_alias-fs.patch
(for the patchbot)
comment:8 Changed 9 years ago by
- Description modified (diff)
comment:9 Changed 9 years ago by
Note that the patches attached to this ticket are newer than the patches on the sage-combinat queue.
Also, I'm in the process of updating this patch with improvements suggested by Mike. New version will be available soon.
comment:10 Changed 9 years ago by
- Reviewers changed from Chris Berg to Chris Berg, Mike Zabrocki
Here is a patch (trac_13243-review_changes-fs.patch), which should be applied on top of the earlier patch, that includes improvements suggested by Mike.
Patchbot:
Apply: trac_13243-new_methods_for_compositions-fs.patch, trac_13243-rebased_deprecated_function_alias-fs.patch, trac_13243-review_changes-fs.patch, trac_13243-documentation_fixes-fs.patch
comment:11 Changed 9 years ago by
- Status changed from needs_review to needs_work
comment:12 Changed 9 years ago by
- Status changed from needs_work to needs_review
comment:13 Changed 9 years ago by
Hello Mike,
Here is a final patch fixing some the issues with the documentation. The documentation now builds and renders nicely. Thanks for your suggestions.
I made separate patches so that it is easier for you to look at my recent changes (but if you prefer, I can post one patch). You only need to look at the following two files to see the changes I made after your review of the first patch:
- trac_13243-review_changes-fs.patch
- trac_13243-documentation_fixes-fs.patch
Franco
comment:14 Changed 9 years ago by
- Status changed from needs_review to needs_work
- Work issues set to documentation improvements
Mike suggested some more documentation improvements. New patch on the way.
comment:15 Changed 9 years ago by
- Status changed from needs_work to needs_review
Here is a patch with improvements to the documentation of shuffle product.
Patchbot:
Apply: trac_13243-new_methods_for_compositions-fs.patch, trac_13243-rebased_deprecated_function_alias-fs.patch, trac_13243-review_changes-fs.patch, trac_13243-documentation_fixes-fs.patch, trac_13243-documentation_fixes_2-fs.patch
comment:16 Changed 9 years ago by
- Work issues documentation improvements deleted
comment:17 follow-up: ↓ 18 Changed 9 years ago by
- Status changed from needs_review to positive_review
comment:18 in reply to: ↑ 17 Changed 9 years ago by
Hi Franco,
You promised to import these patches also to the sage-combinat queue, since the current patch there completely breaks sage-5.2.rc0. I am confused as to which patches are supposed to be applied. For example trac_13243-rebased_deprecated_function_alias-fs.patch also has a newer version trac_13243-rebased_deprecated_function_alias-fs.2.patch, but this is not the one listed in your list.
Thanks,
Anne
comment:19 follow-up: ↓ 21 Changed 9 years ago by
Please clarify which patches have to be applied.
comment:20 Changed 9 years ago by
- Status changed from positive_review to needs_info
comment:21 in reply to: ↑ 19 ; follow-up: ↓ 25 Changed 9 years ago by
Replying to jdemeyer:
Please clarify which patches have to be applied.
If I understand correctly, the patchbot doesn't look at the description for the "apply" directive, so I put it in the comments. Should I also include the directive in the description? Is there a preferred syntax for this?
Apply: trac_13243-new_methods_for_compositions-fs.patch, trac_13243-rebased_deprecated_function_alias-fs.patch, trac_13243-review_changes-fs.patch, trac_13243-documentation_fixes-fs.patch, trac_13243-documentation_fixes_2-fs.patch
For humans:
comment:22 Changed 9 years ago by
- Status changed from needs_info to needs_review
comment:23 Changed 9 years ago by
- Status changed from needs_review to positive_review
comment:24 Changed 9 years ago by
- Description modified (diff)
comment:25 in reply to: ↑ 21 ; follow-up: ↓ 26 Changed 9 years ago by
Replying to saliola:
If I understand correctly, the patchbot doesn't look at the description for the "apply" directive, so I put it in the comments.
I think that's correct yes.
Should I also include the directive in the description?
Yes please!
comment:26 in reply to: ↑ 25 Changed 9 years ago by
Replying to jdemeyer:
Replying to saliola:
If I understand correctly, the patchbot doesn't look at the description for the "apply" directive, so I put it in the comments.
I think that's correct yes.
Should I also include the directive in the description?
Yes please!
Thank you. I'll make sure to do this from now on!
comment:27 Changed 9 years ago by
- Merged in set to sage-5.3.beta0
- Resolution set to fixed
- Status changed from positive_review to closed
Need to fix a dependency error. Patch coming 'soon'.