Ticket #5255 (closed defect: fixed)
[with patch, positive review] Deprecating the use of iterator in CombinatorialClass
| Reported by: | hivert | Owned by: | mhansen |
|---|---|---|---|
| Priority: | major | Milestone: | sage-3.3 |
| Component: | combinatorics | Keywords: | __iter__, iterator |
| Cc: | sage-combinat | Work issues: | |
| Report Upstream: | Reviewers: | ||
| Authors: | Merged in: | ||
| Dependencies: | Stopgaps: |
Description
Right now, when one want's to iterate along a combinatorial class C, there is at least three solution:
[ x for x in C.iterator() ] [ x for x in C.__iter__() ] [ x for x in C ]
There is no semantic differences beetween these three and there should not be any mesurable speedup for any. The latter solution is sintactically better and perfectly python/Sage idiomatic. So the goal of this patch is to mark the use of C.iterator() as deprecated *ASAP* (there are already 96 definition and something close to 400 uses in sage-combinat).
A subsequent series of patches should apply this rule trough all combinatorial classes. Right now to avoid breaking doctests the raising of the deprecation warning is commented out. I'll uncomment it after the series of patches.
Attachments
-
combinatorialclass__iter_-5255-submitted.patch
(8.1 KB) -
added by hivert 4 years ago.
- Patch proposal
-
trac_5255-review.patch
(2.9 KB) -
added by mhansen 4 years ago.
Change History
Changed 4 years ago by hivert
-
attachment
combinatorialclass__iter_-5255-submitted.patch
added
comment:2 Changed 4 years ago by malb
I think
it = C.__iter__() # indirect doctest
should be
it = iter(C) # indirect doctest
i.e. one should avoid calling __foo__ functions directly.
comment:3 follow-up: ↓ 4 Changed 4 years ago by mhansen
The original patch looks good. I made a few updates in the review patch.
comment:4 in reply to: ↑ 3 Changed 4 years ago by hivert
Replying to mhansen:
The original patch looks good. I made a few updates in the review patch.
The review patch looks good to me.
Patch proposal