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

Add subset and superset methods to Sage's finite sets (Set_object_enumerated) #10938

Closed
nthiery opened this issue Mar 15, 2011 · 9 comments
Closed

Comments

@nthiery
Copy link
Contributor

nthiery commented Mar 15, 2011

From the doc::

            sage: X = Set([1,3,5])
            sage: Y = Set([0,1,2,3,5,7])
            sage: X.issubset(Y)
            True
            sage: Y.issupset(X)
            True

Component: combinatorics

Author: Nicolas M. Thiéry

Reviewer: Jason Bandlow

Merged: sage-4.7.1.alpha0

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

@jbandlow
Copy link

comment:1

Hi Nicolas!

One point: I think these should be called is_subset and is_superset for consistency with the most of the other is_* methods throughout sage. I realize that Python sets use issubset and issuperset, but I think the underscore makes the methods easier to read in the long lists that tend to come up with tab completion on sage objects. What do you think?

@nthiery
Copy link
Contributor Author

nthiery commented Mar 15, 2011

comment:2

Hi Jason!

Replying to @jbandlow:

One point: I think these should be called is_subset and is_superset for consistency with the most of the other is_* methods throughout sage. I realize that Python sets use issubset and issuperset, but I think the underscore makes the methods easier to read in the long lists that tend to come up with tab completion on sage objects. What do you think?

Yeah, that's annoying. However, my feeling is that compatibility with
Python's sets should have priority, in particular so that one could
use them a bit more interchangeably (e.g. plug a Sage Set in code that
expects a sage set).

Cheers,
Nicolas

@nthiery nthiery added this to the sage-4.7 milestone Mar 15, 2011
@novoselt
Copy link
Member

comment:3

Would it be acceptable to have both options?

It would be nice also to have a better Hg-comment in the patch ;-)

@nthiery
Copy link
Contributor Author

nthiery commented Mar 16, 2011

comment:4

Attachment: trac_10938-Set-issubset_issuperset-nt.patch.gz

Hi Andrei,

Replying to @novoselt:

Would it be acceptable to have both options?

You are welcome to raise the question on sage-devel. However, that
risks to be a source of confusion in the long run. Typically, a
specific sage set might forget to override both issubset and is_subset
when implementing e.g. a faster method for testing containment. In any cases,
I would tend to leave that to a subsequent patch.

It would be nice also to have a better Hg-comment in the patch ;-)

Oops, fixed. Thanks!

@nthiery
Copy link
Contributor Author

nthiery commented Mar 26, 2011

comment:5

Hi Andrei, Jason!

Could any of you finish the review of this patch to get it in 4.7 and out of the Sage-Combinat queue? (the poset patch depends on it)

Thanks!
Nicolas

@jbandlow
Copy link

Reviewer: Jason Bandlow

@jbandlow
Copy link

comment:6

Ok, I agree that compatibility with Python is the way to go. The patch looks good to me... positive review.

@nthiery
Copy link
Contributor Author

nthiery commented Mar 26, 2011

comment:7

Replying to @jbandlow:

Ok, I agree that compatibility with Python is the way to go. The patch looks good to me... positive review.

Thanks Jason!

@jdemeyer jdemeyer modified the milestones: sage-4.7, sage-4.7.1 Apr 12, 2011
@jdemeyer
Copy link

Merged: sage-4.7.1.alpha0

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

4 participants