#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) Commit: 4d7aa64eec851873106dbd14d0944b9bcda8271b
Dependencies: Stopgaps:

Description


Change History (9)

comment:1 Changed 15 months ago by embray

  • Status changed from new to needs_review

comment:2 Changed 15 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:3 Changed 15 months ago by vbraun

  • 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 15 months ago by git

  • Commit changed from 33e38b4c80b32aea35d88624b6483bf61c01127f to 4d7aa64eec851873106dbd14d0944b9bcda8271b

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4d7aa64py3: fix this example to ensure it works consistently on python 2 and 3

comment:5 follow-up: Changed 15 months ago by 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.

comment:6 Changed 15 months ago by embray

  • Status changed from needs_work to positive_review

comment:7 in reply to: ↑ 5 Changed 15 months ago by tscrim

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 15 months ago by embray

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 15 months ago by vbraun

  • Branch changed from u/embray/python3/sage-rings-polynomial-polydict/misc to 4d7aa64eec851873106dbd14d0944b9bcda8271b
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.