py3: fixes for doctests in cpython

[fchapoton]

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

[fchapoton]

Commit: fb4d072

[fchapoton]

Branch: u/chapoton/26855

[fchapoton]

New commits:

fb4d072

py3: fixes for doctests of cpython

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:2

LGTM.

[jdemeyer]

comment:3

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.

[jdemeyer]

comment:4

General rule about fixing Python 3 doctests: don't "fix" the doctest but fix the code such that the doctest remains working.

[fchapoton]

comment:5

because in python 3, the hash must be a python int...

[jdemeyer]

comment:6

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.

[fchapoton]

comment:7

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

[jdemeyer]

comment:8

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.

[tscrim]

comment:9

My thought was that the coercion (conversion?) from Integer -> int was somehow not done automatically in Python3 but it was allowed in Python2.

[jdemeyer]

comment:10

Replying to @tscrim:

My thought was that the coercion (conversion?) from Integer -> int was 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.

[jdemeyer]

comment:11

Possibly we are doing something wrong in integer.pyx...

[jdemeyer]

Upstream: Reported upstream. No feedback yet.

[jdemeyer]

Changed branch from u/chapoton/26855 to u/jdemeyer/26855

[jdemeyer]

comment:14

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

[jdemeyer]

Changed commit from fb4d072 to 8dde842

[jdemeyer]

Changed upstream from Reported upstream. No feedback yet. to Fixed upstream, but not in a stable release.

[vbraun]

Changed branch from u/jdemeyer/26855 to 8dde842

[embray]

comment:16

This tickets were closed as fixed after the Sage 8.5 release.