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:

Status badges

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)

trac_10938-Set-issubset_issuperset-nt.patch (2.2 KB) - added by nthiery 10 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 follow-up: Changed 10 years ago by jbandlow

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?

comment:2 in reply to: ↑ 1 Changed 10 years ago by nthiery

  • 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: Changed 10 years ago by novoselt

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 nthiery

comment:4 in reply to: ↑ 3 Changed 10 years ago by nthiery

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 nthiery

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: Changed 10 years ago by jbandlow

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

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 jdemeyer

  • Milestone changed from sage-4.7 to sage-4.7.1

comment:9 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.7.1.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.