#5200 closed enhancement (fixed)
[with patch, positive review] subsets and subwords bug fix + improvements.
Reported by: | hivert | Owned by: | hivert |
---|---|---|---|
Priority: | major | Milestone: | sage-3.4.1 |
Component: | combinatorics | Keywords: | subsets, subwords |
Cc: | sage-combinat | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
This patches improves subwords and subsets and deals with several issues:
- It implements subsets for finite multisets (sets with repetitions).
sage: Subsets([2,2,3], multiset=True).list() [[], [2], [3], [2, 2], [2, 3], [2, 2, 3]]
- It implement
__contains__
which was missing for subsets and subwords: Before:sage: st = Subsets([1,2,2,3]); Set([1,2]) in st --------------------------------------------------------------------------- NotImplementedError Traceback (most recent call last)
After:sage: st = Subsets([1,2,2,3]); Set([1,2]) in st True
- It fixes a bug in smallest_positions:
Before:
sage: sage.combinat.subword.smallest_positions([2,4,3,3,1,2],[1,3,3]) [4, 4, 4]
After:sage.combinat.subword.smallest_positions([2,4,3,3,1,2],[1,3,3]) False
which means that 113 is not a subword of 243312. - It finally improves the doc and the tests.
Cheers,
Florent
Attachments (2)
Change History (15)
comment:1 Changed 13 years ago by
- Summary changed from [with patch, needs review] subsets and subwords bug fix + improvements. to [with patch, needs review, doc needs work] subsets and subwords bug fix + improvements.
comment:2 Changed 13 years ago by
comment:3 Changed 13 years ago by
- Description modified (diff)
- Owner changed from mhansen to hivert
- Summary changed from [with patch, needs review, doc needs work] subsets and subwords bug fix + improvements. to [with patch, needs review] subsets and subwords bug fix + improvements.
comment:4 Changed 13 years ago by
- Status changed from new to assigned
comment:5 Changed 13 years ago by
- Description modified (diff)
comment:6 Changed 13 years ago by
Sorry for the mess with the description... I just realize because of my mistake that it is possible to change the description. :-)
I've uploaded a new patch according to remark of Nicolas and Mike. It should be ready for review and hopefully integration.
Cheers,
Florent
comment:7 Changed 13 years ago by
Nicolas sent me a review on sage-combinat-devel. The new subsets_subwords-5200-submitted.2.patch patch fixes the various problems pointed out by the review (typos + code improvements).
Florent
comment:8 Changed 13 years ago by
Florent: feel free to switch the title to with review after the following micro updates:
- iterate -> iterates
- of all -> ""
comment:9 Changed 13 years ago by
- Summary changed from [with patch, needs review] subsets and subwords bug fix + improvements. to [with patch, with review] subsets and subwords bug fix + improvements.
The review patch I just submitted address Nicolas last comment. Sorry for the mess with several version of the patch. The correct patch are:
According to Nicolas la remarks I allow myself to change the review title. Please tell me if it's not allowed by the review rules.
Florent
comment:10 Changed 13 years ago by
- Summary changed from [with patch, with review] subsets and subwords bug fix + improvements. to [with patch, needs review] subsets and subwords bug fix + improvements.
"with review" is meaningless :)
Nicolas: Can you give this a formal positive review?
Cheers,
Michael
comment:11 Changed 13 years ago by
- Summary changed from [with patch, needs review] subsets and subwords bug fix + improvements. to [with patch, positive review] subsets and subwords bug fix + improvements.
After re-reading Nicola's comment I am giving this patch a positive review in his name. The two patches also pass doctests.
Cheers,
Michael
comment:12 Changed 13 years ago by
- Milestone changed from sage-combinat to sage-3.4.1
- Resolution set to fixed
- Status changed from assigned to closed
Merged both patches in Sage 3.4.1.alpha0.
Cheers,
Michael
comment:13 Changed 13 years ago by
- Cc sage-combinat added
It has been decided (direct talk with Nicolas + irssi with Mike) that the user has to explicitely set that he want multisets. Therefore, on the contrary that is anounced, the following is not working:
Instead one should write