Opened 10 years ago
Closed 10 years ago
#10938 closed enhancement (fixed)
Add subset and superset methods to Sage's finite sets (Set_object_enumerated)
Reported by: | nthiery | Owned by: | sage-combinat |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7.1 |
Component: | combinatorics | Keywords: | |
Cc: | Merged in: | sage-4.7.1.alpha0 | |
Authors: | Nicolas M. Thiéry | Reviewers: | Jason Bandlow |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
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
Attachments (1)
Change History (10)
comment:1 follow-up: ↓ 2 Changed 10 years ago by
comment:2 in reply to: ↑ 1 Changed 10 years ago by
- Milestone set to sage-4.7
- Status changed from new to needs_review
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
comment:3 follow-up: ↓ 4 Changed 10 years ago by
Would it be acceptable to have both options?
It would be nice also to have a better Hg-comment in the patch ;-)
Changed 10 years ago by
comment:4 in reply to: ↑ 3 Changed 10 years ago by
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!
comment:5 Changed 10 years ago by
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
comment:6 follow-up: ↓ 7 Changed 10 years ago by
- Reviewers set to Jason Bandlow
- Status changed from needs_review to positive_review
Ok, I agree that compatibility with Python is the way to go. The patch looks good to me... positive review.
comment:7 in reply to: ↑ 6 Changed 10 years ago by
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!
comment:8 Changed 10 years ago by
- Milestone changed from sage-4.7 to sage-4.7.1
comment:9 Changed 10 years ago by
- Merged in set to sage-4.7.1.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
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?