py3: fixes for doctests in cpython
Closed this issue · 22 comments
Reported upstream: cython/cython#2752
Upstream: Fixed upstream, but not in a stable release.
Component: python3
Author: Frédéric Chapoton
Branch/Commit: 8dde842
Reviewer: Travis Scrimshaw
Issue created by migration from https://trac.sagemath.org/ticket/26855
Branch: u/chapoton/26855
Reviewer: Travis Scrimshaw
LGTM.
Why is this needed?
- sage: test_del_dictitem_by_exact_value(D, ZZ, 2)
+ sage: test_del_dictitem_by_exact_value(D, ZZ, int(2))If this change is really needed, I consider it a bug in Cython.
General rule about fixing Python 3 doctests: don't "fix" the doctest but fix the code such that the doctest remains working.
because in python 3, the hash must be a python int...
Replying to @fchapoton:
because in python 3, the hash must be a python int...
What do you mean? Are you talking about the output of a plain-Python __hash__ method? This is not involved here.
sage -t --long src/sage/cpython/dict_del_by_value.pyx
**********************************************************************
File "src/sage/cpython/dict_del_by_value.pyx", line 435, in sage.cpython.dict_del_by_value.test_del_dictitem_by_exact_value
Failed example:
test_del_dictitem_by_exact_value(D, ZZ, 2)
Exception raised:
Traceback (most recent call last):
File "/home/u1/chapoton/sage3/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 671, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/u1/chapoton/sage3/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
exec(compiled, globs)
File "<doctest sage.cpython.dict_del_by_value.test_del_dictitem_by_exact_value[7]>", line 1, in <module>
test_del_dictitem_by_exact_value(D, ZZ, Integer(2))
File "sage/cpython/dict_del_by_value.pyx", line 443, in sage.cpython.dict_del_by_value.test_del_dictitem_by_exact_value (build/cythonized/sage/cpython/dict_del_by_value.c:2504)
return del_dictitem_by_exact_value(<PyDictObject *>D, <PyObject *>value, h)
TypeError: an integer is required
I'm assuming that this is an exception raised by Cython. So I would like to understand why it works on Python 2 but not on Python 3. This is not only going to be relevant for this ticket, but possibly for many other Python 3 problems regarding Sage Integer vs Python int.
My thought was that the coercion (conversion?) from Integer -> int was somehow not done automatically in Python3 but it was allowed in Python2.
Replying to @tscrim:
My thought was that the coercion (conversion?) from
Integer->intwas somehow not done automatically in Python3 but it was allowed in Python2.
I think it's at least something that we should investigate in general.
Possibly we are doing something wrong in integer.pyx...
Upstream: Reported upstream. No feedback yet.
Description changed:
---
+++
@@ -1 +1 @@
-
+Reported upstream: https://github.com/cython/cython/issues/2752Changed branch from u/chapoton/26855 to u/jdemeyer/26855
The problem with Py_hash_t should be fixed upstream in Cython. So I will revert this part of this ticket, which should be solved whenever we upgrade Cython.
New commits:
8dde842 | py3: fixes for doctests of cpython |
Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.
Changed branch from u/jdemeyer/26855 to 8dde842
This tickets were closed as fixed after the Sage 8.5 release.