Opened 13 years ago

Closed 13 years ago

[with patch, positive review] subsets and subwords bug fix + improvements.

Reported by: Owned by: hivert hivert major sage-3.4.1 combinatorics subsets, subwords sage-combinat

This patches improves subwords and subsets and deals with several issues:

1. 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]]
```
2. 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
```
3. 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.
4. It finally improves the doc and the tests.

Cheers,

Florent

comment:1 Changed 13 years ago by hivert

• 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 hivert

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:

```sage: Subsets([2,2,3]).list()
[[], [2], [3], [2, 2], [2, 3], [2, 2, 3]]
```

```sage: Subsets([2,2,3], multiset=True).list()
[[], [2], [3], [2, 2], [2, 3], [2, 2, 3]]
```

comment:3 Changed 13 years ago by hivert

• 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 hivert

• Status changed from new to assigned

comment:5 Changed 13 years ago by hivert

• Description modified (diff)

comment:6 Changed 13 years ago by hivert

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

Changed 13 years ago by hivert

New patch after Nicolas review.

comment:7 Changed 13 years ago by hivert

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 nthiery

Florent: feel free to switch the title to with review after the following micro updates:

• iterate -> iterates
• of all -> ""

Changed 13 years ago by hivert

Last review patch.

comment:9 Changed 13 years ago by hivert

• 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 mabshoff

• 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 mabshoff

• 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 mabshoff

• 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