Opened 11 years ago
Closed 10 years ago
#8925 closed enhancement (fixed)
__call__ for categories sets / enumeratedsets
Reported by: | hivert | Owned by: | hivert |
---|---|---|---|
Priority: | major | Milestone: | sage-4.6.2 |
Component: | categories | Keywords: | Category call |
Cc: | Merged in: | sage-4.6.2.alpha0 | |
Authors: | Florent Hivert | Reviewers: | Mike Hansen, Jason Grout |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
From sets_cat.py
:
FIXME: the above behavior dates back from the first category writeup. It is not consistent with :meth:`Category.__call__`. Should we change it to just return ``ZZ`` instead?
Also EnumeratedSets().__call__(...)
is missing.
Attachments (1)
Change History (13)
comment:1 Changed 11 years ago by
- Status changed from new to needs_review
comment:2 follow-up: ↓ 3 Changed 10 years ago by
- Reviewers set to Mike Hansen
- Status changed from needs_review to needs_work
comment:3 in reply to: ↑ 2 Changed 10 years ago by
- Status changed from needs_work to needs_review
Replying to mhansen:
Looks good except for some minor formatting issues. I put a review patch up on the patch server. Do you want to fold that in and reupload?
Done ! I'm ok with your changes. Feel free to put a positive review if you are ok.
comment:4 follow-up: ↓ 6 Changed 10 years ago by
- Status changed from needs_review to positive_review
comment:5 Changed 10 years ago by
- Milestone changed from sage-4.6.1 to sage-4.6.2
comment:6 in reply to: ↑ 4 Changed 10 years ago by
Replying to mhansen:
Thanks for the review !
comment:7 follow-up: ↓ 8 Changed 10 years ago by
Extremely picky, minor, minor thing, but these words: "For now, list's, tuple's, set's, Set's are coerced into finite" shouldn't have apostrophes in them, since the words are not possessive, but just plural.
comment:8 in reply to: ↑ 7 ; follow-up: ↓ 9 Changed 10 years ago by
- Status changed from positive_review to needs_work
Replying to jason:
Extremely picky, minor, minor thing, but these words: "For now, list's, tuple's, set's, Set's are coerced into finite" shouldn't have apostrophes in them, since the words are not possessive, but just plural.
I reuploaded a new patche with the following diff folded:
diff --git a/sage/categories/enumerated_sets.py b/sage/categories/enumerated_sets.py --- a/sage/categories/enumerated_sets.py +++ b/sage/categories/enumerated_sets.py @@ -103,7 +103,7 @@ class EnumeratedSets(Category): sage: EnumeratedSets()(Primes()) Set of all prime numbers: 2, 3, 5, 7, ... - For now, list's, tuple's, set's, Set's are coerced into finite + For now, lists, tuples, sets, Sets are coerced into finite enumerated sets:: sage: S = EnumeratedSets()([1, 2, 3]); S
So now you have to review it ;-)
Changed 10 years ago by
comment:9 in reply to: ↑ 8 Changed 10 years ago by
- Status changed from needs_work to needs_review
Replying to hivert:
Replying to jason:
Extremely picky, minor, minor thing, but these words: "For now, list's, tuple's, set's, Set's are coerced into finite" shouldn't have apostrophes in them, since the words are not possessive, but just plural.
I reuploaded a new patche with the following diff folded:
Actually there where another occurrence in another file
diff --git a/trac_8925-call_set_enumset-fh.patch b/trac_8925-call_set_enumset-fh.patch --- a/trac_8925-call_set_enumset-fh.patch +++ b/trac_8925-call_set_enumset-fh.patch @@ -83,7 +83,7 @@ diff --git a/sage/categories/finite_enum + sage: FiniteEnumeratedSets()(Partitions(3)) # todo: not implemented: Partitions + Partitions of 3 + -+ For now, list's, tuple's, set's, Set's are coerced into finite ++ For now, lists, tuples, sets, Sets are coerced into finite + enumerated sets:: + + sage: FiniteEnumeratedSets()([1, 2, 3])
Again I folded this change and resubmitted the patch. Please review.
comment:10 Changed 10 years ago by
- Reviewers changed from Mike Hansen to Mike Hansen, Jason Grout
- Status changed from needs_review to positive_review
Looks good.
comment:11 Changed 10 years ago by
Looks good to me too. Thanks for working on even the most minor nitpicky requests :).
comment:12 Changed 10 years ago by
- Merged in set to sage-4.6.2.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
Looks good except for some minor formatting issues. I put a review patch up on the patch server. Do you want to fold that in and reupload?