Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subsets and subwords bug fix + improvements. #5200

Closed
hivert opened this issue Feb 7, 2009 · 12 comments
Closed

subsets and subwords bug fix + improvements. #5200

hivert opened this issue Feb 7, 2009 · 12 comments

Comments

@hivert
Copy link

hivert commented Feb 7, 2009

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

Cheers,

Florent

CC: @sagetrac-sage-combinat

Component: combinatorics

Keywords: subsets, subwords

Issue created by migration from https://trac.sagemath.org/ticket/5200

@hivert hivert changed the title subsets and subwords bug fix + improvements. [doc needs work] subsets and subwords bug fix + improvements. Feb 18, 2009
@hivert
Copy link
Author

hivert commented Mar 1, 2009

comment:2

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]]

Instead one should write

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

@hivert

This comment has been minimized.

@hivert hivert changed the title [doc needs work] subsets and subwords bug fix + improvements. subsets and subwords bug fix + improvements. Mar 1, 2009
@hivert hivert assigned hivert and unassigned mwhansen Mar 1, 2009
@hivert

This comment has been minimized.

@hivert
Copy link
Author

hivert commented Mar 1, 2009

comment:6

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

@hivert
Copy link
Author

hivert commented Mar 17, 2009

New patch after Nicolas review.

@hivert
Copy link
Author

hivert commented Mar 17, 2009

comment:7

Attachment: subsets_subwords-5200-submitted.2.patch.gz

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

@nthiery
Copy link
Contributor

nthiery commented Mar 19, 2009

comment:8

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

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

@hivert
Copy link
Author

hivert commented Mar 19, 2009

Attachment: subsets_subwords-5200-review.patch.gz

Last review patch.

@hivert
Copy link
Author

hivert commented Mar 19, 2009

comment:9

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

@hivert hivert changed the title subsets and subwords bug fix + improvements. [with review] subsets and subwords bug fix + improvements. Mar 19, 2009
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Mar 25, 2009

comment:10

"with review" is meaningless :)

Nicolas: Can you give this a formal positive review?

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin changed the title [with review] subsets and subwords bug fix + improvements. subsets and subwords bug fix + improvements. Mar 25, 2009
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Mar 25, 2009

comment:11

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

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Mar 25, 2009

comment:12

Merged both patches in Sage 3.4.1.alpha0.

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Mar 25, 2009
@sagetrac-mabshoff sagetrac-mabshoff mannequin added this to the sage-3.4.1 milestone Mar 25, 2009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants