Opened 2 years ago
Closed 2 years ago
#26280 closed defect (fixed)
py3: more trivial fixes to sage.rings.polynomial.polydict
Reported by:  embray  Owned by:  

Priority:  trivial  Milestone:  sage8.4 
Component:  python3  Keywords:  
Cc:  Merged in:  
Authors:  Erik Bray  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  4d7aa64 (Commits)  Commit:  4d7aa64eec851873106dbd14d0944b9bcda8271b 
Dependencies:  Stopgaps: 
Description
Change History (9)
comment:1 Changed 2 years ago by
 Status changed from new to needs_review
comment:2 Changed 2 years ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
comment:3 Changed 2 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/sagepatchbot/sage/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 650, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/sagepatchbot/sage/local/lib/python2.7/sitepackages/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 2 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 followup: ↓ 7 Changed 2 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 2 years ago by
 Status changed from needs_work to positive_review
comment:7 in reply to: ↑ 5 Changed 2 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 2 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 2 years ago by
 Branch changed from u/embray/python3/sageringspolynomialpolydict/misc to 4d7aa64eec851873106dbd14d0944b9bcda8271b
 Resolution set to fixed
 Status changed from positive_review to closed
LGTM.