Opened 7 years ago

Closed 7 years ago

#18578 closed enhancement (fixed)

Python 3 preparation: Special function __div__() is used no more in Py3

Reported by: Wilfried Luebbe Owned by: Wilfried Luebbe
Priority: major Milestone: sage-7.0
Component: misc Keywords: python3
Cc: Merged in:
Authors: Wilfried Luebbe, Jeroen Demeyer Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: 1d0d015 (Commits, GitHub, GitLab) Commit: 1d0d01573f63702e802b59633d17ae8dc78bbb98
Dependencies: Stopgaps:

Status badges

Description

There (among others) the three special functions __div__(), __truediv__() and __floordiv__().

They are invoked by the division operators / and //:

  • In Py2: __div__() and __floordiv__() are called.
  • In Py2 (when from __future__ import division is in effect): __truediv__() and __floordiv__() are called.
  • In Py3: __truediv__() and __floordiv__() are called.

So in the last two cases __div__() is not called for / but __truediv__(). If __truediv__() is not available an exception is thrown.

Therefor classes defining __div__() must be checked that they also work in the last two cases! The related special function __idiv__() is effected too.

The ticket is tracked as a dependency of #15995.

Attachments (2)

test__div__.py (2.1 KB) - added by Wilfried Luebbe 7 years ago.
Script illustrating div() et.al. with Py2 (and from future import division) and Py3
grep-of-div__.txt (11.2 KB) - added by Wilfried Luebbe 7 years ago.
The result of git grep -P "div" >~/grep-of-div.txt

Download all attachments as: .zip

Change History (39)

Changed 7 years ago by Wilfried Luebbe

Attachment: test__div__.py added

Script illustrating div() et.al. with Py2 (and from future import division) and Py3

comment:1 Changed 7 years ago by Wilfried Luebbe

To observe the actual code behavior (besides reading the Python docs) I created a small test script (see the attached file). A summary of the output under various conditions is here:

__div__       is defined
__truediv__   is defined
__floordiv__  is defined
__idiv__      is defined
__itruediv__  is defined
__ifloordiv__ is defined

python version: 2.7.10 (default, May 23 2015, 09:44:00) [MSC v.1500 64 bit (AMD64)]
from __future__ import division is NOT active
function: __div__       115 10 ## operator is /; result=cla(11)
function: __floordiv__  115 10 ## operator is //; result=cla(11)
function: __idiv__      215 10 ## operator is /=; result=cla(21)
function: __ifloordiv__ 215 10 ## operator is //=; result=cla(21)

Python version: 2.7.10 (default, May 23 2015, 09:44:00) [MSC v.1500 64 bit (AMD64)]
from __future__ import division is active
function: __truediv__   115 10 ## operator is /; result=cla(11.5)
function: __floordiv__  115 10 ## operator is //; result=cla(11)
function: __itruediv__  215 10 ## operator is /=; result=cla(21.5)
function: __ifloordiv__ 215 10 ## operator is //=; result=cla(21)

Python version: 3.4.3 (v3.4.3:9b73f1c3e601, Feb 24 2015, 22:44:40) [MSC v.1600 64 bit (AMD64)]
from __future__ import division is NOT active
function: __truediv__   115 10 ## operator is /; result=cla(11.5)
function: __floordiv__  115 10 ## operator is //; result=cla(11)
function: __itruediv__  215 10 ## operator is /=; result=cla(21.5)
function: __ifloordiv__ 215 10 ## operator is //=; result=cla(21)

Python version: 3.4.3 (v3.4.3:9b73f1c3e601, Feb 24 2015, 22:44:40) [MSC v.1600 64 bit (AMD64)]
from __future__ import division is active
function: __truediv__   115 10 ## operator is /; result=cla(11.5)
function: __floordiv__  115 10 ## operator is //; result=cla(11)
function: __itruediv__  215 10 ## operator is /=; result=cla(21.5)
function: __ifloordiv__ 215 10 ## operator is //=; result=cla(21)

Changed 7 years ago by Wilfried Luebbe

Attachment: grep-of-div__.txt added

The result of git grep -P "div" >~/grep-of-div.txt

comment:2 Changed 7 years ago by Wilfried Luebbe

Branch: u/wluebbe/18578
Commit: 9701674d59213529ef6476ecc973a8f2cf69bfb3
Owner: set to Wilfried Luebbe

Currently the branch contains the changes of 20 .py modules. I start working on the .pyx modules now.


New commits:

9701674Trac #18578: Add special function __truediv__() for Py3

comment:3 Changed 7 years ago by Wilfried Luebbe

But trying the same approach does not seem to work for (e.g.) src/sage/ext/fast_callable.pyx.

    def __truediv__(s, o):
    ...
    ...
    # for Python 2 without from __future__ import division
    __div__ = __truediv__

apparently does not define __div__().

comment:4 in reply to:  3 Changed 7 years ago by Jeroen Demeyer

Replying to wluebbe:

But trying the same approach does not seem to work for (e.g.) src/sage/ext/fast_callable.pyx.

    def __truediv__(s, o):
    ...
    ...
    # for Python 2 without from __future__ import division
    __div__ = __truediv__

apparently does not define __div__().

A cdef class in Cython is really a different thing than a Python class. So it's normal that the same approach doesn't work.

You could (and should) use the __div__ = __truediv__ trick for a class defined in Cython code.

comment:5 Changed 7 years ago by Jeroen Demeyer

How about this in Cython?

from cpython.number cimport PyNumber_TrueDivide

def __div__(self, other):
    return PyNumber_TrueDivide(self, other)

comment:6 Changed 7 years ago by Wilfried Luebbe

That gives a compile error:

Cythonizing sage/ext/fast_callable.pyx

Error compiling Cython file:
------------------------------------------------------------
...
            div(1, v_0)
        """
        return _expression_binop_helper(s, o, op_div)

    # for Python 2 without from __future__ import division
    from cpython.number cimport PyNumber_TrueDivide
   ^
------------------------------------------------------------

sage/ext/fast_callable.pyx:917:4: cimport only allowed at module level

Error compiling Cython file:
------------------------------------------------------------
...
        return _expression_binop_helper(s, o, op_div)

    # for Python 2 without from __future__ import division
    from cpython.number cimport PyNumber_TrueDivide
    def __div__(self, other):
        return PyNumber_TrueDivide(self, other)
                                 ^
------------------------------------------------------------

sage/ext/fast_callable.pyx:919:34: undeclared name not builtin: PyNumber_TrueDivide

Copying the code from def __div__ to def ___truediv__ clearly works - but there must be a better way!

comment:7 in reply to:  6 Changed 7 years ago by Jeroen Demeyer

Replying to wluebbe:

That gives a compile error:

sage/ext/fast_callable.pyx:917:4: cimport only allowed at module level

Well, that's because you need to write

... cimport ...

cdef class ...

instead of

cdef class ...

    ... cimport ...

comment:8 Changed 7 years ago by Wilfried Luebbe

Yes, that did the trick.

I was thinking that I could do without learning Cython ... but ... sigh ...

Thanks for the help!

comment:9 Changed 7 years ago by Wilfried Luebbe

Not a complete success :-( . I am going into a never ending recursion:

    RuntimeError: maximum recursion depth exceeded while calling a Python object
**********************************************************************
1 item had failures:
   2 of   9 in sage.ext.fast_callable.Expression.__truediv__

I am just looking for a way that __div__ directly delegates the work to __truediv__? Calling PyNumber_TrueDivide seems to cause the recursion. How can I call the directly previously defined method __truediv__(s, o) from within def __div__(s,o):?

comment:10 Changed 7 years ago by Jeroen Demeyer

I would be easier if you just show me what you have done.

comment:11 Changed 7 years ago by Wilfried Luebbe

My current changes are in u/wluebbe/18578-1. Meanwhile I will have a look at the Cython doc.

comment:12 in reply to:  9 Changed 7 years ago by Jeroen Demeyer

Replying to wluebbe:

Not a complete success :-( . I am going into a never ending recursion:

    RuntimeError: maximum recursion depth exceeded while calling a Python object
**********************************************************************
1 item had failures:
   2 of   9 in sage.ext.fast_callable.Expression.__truediv__

If you look at the traceback, you see why:

      File "sage/ext/fast_callable.pyx", line 918, in sage.ext.fast_callable.Expression.__div__ (build/cythonized/sage/ext/fast_callable.c:7203)
        return PyNumber_TrueDivide(self, other)
      File "sage/structure/element.pyx", line 2025, in sage.structure.element.RingElement.__truediv__ (build/cythonized/sage/structure/element.c:18064)
        return self.__div__(right)
      File "sage/structure/element.pyx", line 2038, in sage.structure.element.RingElement.__div__ (build/cythonized/sage/structure/element.c:18205)
        return coercion_model.bin_op(self, right, div)
      File "sage/structure/coerce.pyx", line 1040, in sage.structure.coerce.CoercionModel_cache_maps.bin_op (build/cythonized/sage/structure/coerce.c:9052)
        res = mul_method(x)

The problem is caused by this code in element.pyx:

    def __truediv__(self, right):
        # in sage all divs are true
        if not isinstance(self, Element):
            return coercion_model.bin_op(self, right, div)
        return self.__div__(right)

comment:13 Changed 7 years ago by Wilfried Luebbe

I see -- and element.pyx is also on my todo list. But that code is not the easiest to understand :-/

comment:14 Changed 7 years ago by Jeroen Demeyer

Dependencies: #18622

comment:15 Changed 7 years ago by Jeroen Demeyer

In the cases where the objects involved are Parents, I would rather just put

def __div__(self, other):
    return PyNumber_TrueDivide(self, other)

in Parent instead of manually adding __div__ = __truediv__ everywhere.

comment:16 Changed 7 years ago by Jeroen Demeyer

I created #18622 to deal with element.pyx

comment:17 Changed 7 years ago by Wilfried Luebbe

I will work on the other pyx modules with sage-6.8.beta4 (#18622 has just been closed).

comment:18 Changed 7 years ago by git

Commit: 9701674d59213529ef6476ecc973a8f2cf69bfb3a51c0422b3d3a40aaa64ef13329395b840d8ee34

Branch pushed to git repo; I updated commit sha1. New commits:

a51c042Merge branch 'develop' into u/wluebbe/18578

comment:19 Changed 7 years ago by Wilfried Luebbe

Resolved merge conflict with sage-6.8.beta5.

comment:20 Changed 7 years ago by Jeroen Demeyer

Dependencies: #18622

comment:21 Changed 7 years ago by Jeroen Demeyer

When you continue working on this, please keep in mind 15.

comment:22 Changed 7 years ago by Jeroen Demeyer

In src/sage/rings/number_field/number_field_ideal.py, you are replacing _div_ (single underscore!). That's wrong and should not be changed.

comment:23 Changed 7 years ago by git

Commit: a51c0422b3d3a40aaa64ef13329395b840d8ee34610a2c4123e314979e9895700687aeae79f9e185

Branch pushed to git repo; I updated commit sha1. New commits:

699cb26Merge branch 'develop' into u/wluebbe/18578
610a2c4Revert error introduced during last merge

comment:24 in reply to:  21 Changed 7 years ago by Wilfried Luebbe

Replying to jdemeyer:

When you continue working on this, please keep in mind 15.

Yes, I will.

While merging sage-6.8.beta5 I got confused: I did not notice what your change of number_field_ideal.py in #18622 was. And I pushed before testing the merge ... Then I struggled with undoing my mess :-/

comment:25 Changed 7 years ago by git

Commit: 610a2c4123e314979e9895700687aeae79f9e185894192195d46cf6b07a43afe0cbc69a54bd3a7f0

Branch pushed to git repo; I updated commit sha1. New commits:

8941921Trac #18578: Add special function __truediv__() to some pyx-modules

comment:26 Changed 7 years ago by Wilfried Luebbe

Authors: Wilfried Luebbe
Status: newneeds_review

comment:27 Changed 7 years ago by Wilfried Luebbe

Perhaps those tiny new functions might also need doctests?

comment:28 Changed 7 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

See 15

comment:29 Changed 7 years ago by Jeroen Demeyer

In src/sage/rings/polynomial/polynomial_element.pyx, in __truediv__, call RingElement.__truediv__ instead RingElement.__div__.

comment:30 Changed 7 years ago by Jeroen Demeyer

Don't use

PeriodicRegion.__truediv__(self, other)

in Cython. It's much slower than PyNumber_TrueDivide.

comment:31 Changed 7 years ago by Jeroen Demeyer

See also #19535 and #19536 for some partial progress.

comment:32 Changed 7 years ago by Jeroen Demeyer

Authors: Wilfried LuebbeWilfried Luebbe, Jeroen Demeyer
Milestone: sage-6.8sage-7.0

comment:33 Changed 7 years ago by Jeroen Demeyer

Branch: u/wluebbe/18578u/jdemeyer/18578

comment:34 Changed 7 years ago by Jeroen Demeyer

Commit: 894192195d46cf6b07a43afe0cbc69a54bd3a7f01d0d01573f63702e802b59633d17ae8dc78bbb98
Status: needs_workneeds_review

New commits:

1d0d015Trac #18578: Add special function __truediv__() for Py3

comment:35 Changed 7 years ago by Jeroen Demeyer

See also #19842.

comment:36 Changed 7 years ago by Volker Braun

Reviewers: Volker Braun
Status: needs_reviewpositive_review

comment:37 Changed 7 years ago by Volker Braun

Branch: u/jdemeyer/185781d0d01573f63702e802b59633d17ae8dc78bbb98
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.