Opened 4 years ago
Closed 4 years ago
#26280 closed defect (fixed)
py3: more trivial fixes to sage.rings.polynomial.polydict
Reported by: | embray | Owned by: | |
---|---|---|---|
Priority: | trivial | Milestone: | sage-8.4 |
Component: | python3 | Keywords: | |
Cc: | Merged in: | ||
Authors: | Erik Bray | Reviewers: | Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | 4d7aa64 (Commits, GitHub, GitLab) | Commit: | 4d7aa64eec851873106dbd14d0944b9bcda8271b |
Dependencies: | Stopgaps: |
Description
Change History (9)
comment:1 Changed 4 years ago by
- Status changed from new to needs_review
comment:2 Changed 4 years ago by
- Reviewers set to Travis Scrimshaw
- Status changed from needs_review to positive_review
comment:3 Changed 4 years ago by
- Status changed from positive_review to needs_work
File "src/sage/rings/polynomial/polydict.pyx", line 982, in sage.rings.polynomial.polydict.ETuple.__init__ Failed example: ETuple(iter([2, 3, 4])) Expected: Traceback (most recent call last): ... TypeError: Error in ETuple((), <list... object at ...>, None) Got: <BLANKLINE> Traceback (most recent call last): File "/home/sage-patchbot/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 650, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/sage-patchbot/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1061, in compile_and_execute exec(compiled, globs) File "<doctest sage.rings.polynomial.polydict.ETuple.__init__[3]>", line 1, in <module> ETuple(iter([Integer(2), Integer(3), Integer(4)])) File "sage/rings/polynomial/polydict.pyx", line 1007, in sage.rings.polynomial.polydict.ETuple.__init__ (build/cythonized/sage/rings/polynomial/polydict.c:13297) self._length = len(data) TypeError: object of type 'listiterator' has no len()
comment:4 Changed 4 years ago by
- Commit changed from 33e38b4c80b32aea35d88624b6483bf61c01127f to 4d7aa64eec851873106dbd14d0944b9bcda8271b
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
4d7aa64 | py3: fix this example to ensure it works consistently on python 2 and 3
|
comment:5 follow-up: ↓ 7 Changed 4 years ago by
It seems Cython on Python 2 has some strange behaviors if you put isinstance(..., range)
. So I'm just going to drop that change until we have a better, consistent way to accept arbitrary sequence types in function arguments.
comment:6 Changed 4 years ago by
- Status changed from needs_work to positive_review
comment:7 in reply to: ↑ 5 Changed 4 years ago by
Replying to embray:
It seems Cython on Python 2 has some strange behaviors if you put
isinstance(..., range)
. So I'm just going to drop that change until we have a better, consistent way to accept arbitrary sequence types in function arguments.
I would guess that comes from the fact that range
on Cython is treated as special by the compiler as it can both be treated as xrange
(e.g., in a for
loop) and as a list:
sage: %%cython ....: def test(n): ....: return range(n) sage: test(5) [0, 1, 2, 3, 4]
comment:8 Changed 4 years ago by
Well, yes, but I'm not sure how it translates a statement like isinstance(..., range)
and I didn't bother to look, but it seems to get up to some funny business for sure.
comment:9 Changed 4 years ago by
- Branch changed from u/embray/python3/sage-rings-polynomial-polydict/misc to 4d7aa64eec851873106dbd14d0944b9bcda8271b
- Resolution set to fixed
- Status changed from positive_review to closed
LGTM.