gh-130104: Call __rpow__ in ternary pow() if necessary by serhiy-storchaka · Pull Request #130251 · python/cpython

serhiy-storchaka · 2025-02-18T09:54:54Z

Previously it was only called in binary pow() and the binary power operator.

Issue: Inconsistent behavior of ternary pow() for Python classes and C extension types #130104

📚 Documentation preview 📚: https://cpython-previews--130251.org.readthedocs.build/

Previously it was only called in binary pow() and the binary power operator.

Objects/typeobject.c

skirpichev

LGTM (few nitpicks)

Maybe it's possible to refactor code to avoid a copy of SLOT1BINFULL, but I doubt it worth efforts.

skirpichev · 2025-02-19T00:21:19Z

Doc/reference/datamodel.rst

   is a subclass of ``type(x)``. [#]_

-   .. index:: pair: built-in function; pow
+   Note that :meth:`__rpow__` should be defined to accept an optional third


Maybe you should use object.__rpow__(self, other, modulo=None) signature?

I do not understand you. Were and how can it be used?

This sentence is a copy of the corresponding sentence for __pow__.

Sorry, I meant L3329-3342 above.

I see. There are other similar cases (for example __round__), so I will left this for other issue. Actually, __pow__() and __round__() are never called with None, so it is not necessary that they support None.

Doc/reference/datamodel.rst

skirpichev · 2025-02-20T10:36:10Z

CC @picnixz (this is an alternative to #122193)

picnixz

First round of style review only. I'll look at the implementation and implications more in detail either later or tomorrow.

picnixz · 2025-03-02T13:15:39Z

Doc/whatsnew/3.14.rst

+* Three-argument :func:`pow` now try calling :meth:`~object.__rpow__` if necessary.
+  Previously it was only called in two-argument :func:`!pow` and the binary
+  power operator.
+  (Contributed by Serhiy Storchaka in :gh:`130104`.)
+


Suggested change

* Three-argument :func:`pow` now try calling :meth:`~object.__rpow__` if necessary.

Previously it was only called in two-argument :func:`!pow` and the binary

power operator.

(Contributed by Serhiy Storchaka in :gh:`130104`.)

* Three-argument :func:`pow` now try calling :meth:`~object.__rpow__` if necessary.

Previously it was only called in two-argument :func:`!pow` and the binary

power operator.

(Contributed by Serhiy Storchaka in :gh:`130104`.)

What is the difference?

picnixz · 2025-03-02T13:18:07Z

Objects/typeobject.c

@@ -9813,13 +9813,46 @@ slot_nb_power(PyObject *self, PyObject *other, PyObject *modulus)
 {
    if (modulus == Py_None)


Let's insert some PEP-7 here if (modulus == Py_None) { ...

picnixz · 2025-03-02T13:18:53Z

Objects/typeobject.c

+    int do_other = !Py_IS_TYPE(self, Py_TYPE(other)) &&
+        Py_TYPE(other)->tp_as_number != NULL &&
+        Py_TYPE(other)->tp_as_number->nb_power == slot_nb_power;


We use a lot of Py_TYPE(other), so I think we can have a variable holding it just for readability.

This code is a copy of SLOT1BINFULL. I do not want to introduce more difference than necessary.

We can make SLOT1BINFULL supporting the third argument, but I am not sure that it is worth.

picnixz · 2025-03-02T13:19:29Z

Objects/typeobject.c

+    int do_other = !Py_IS_TYPE(self, Py_TYPE(other)) &&
+        Py_TYPE(other)->tp_as_number != NULL &&
+        Py_TYPE(other)->tp_as_number->nb_power == slot_nb_power;


Suggested change

int do_other = !Py_IS_TYPE(self, Py_TYPE(other)) &&

Py_TYPE(other)->tp_as_number != NULL &&

Py_TYPE(other)->tp_as_number->nb_power == slot_nb_power;

int do_other = !Py_IS_TYPE(self, Py_TYPE(other))

&& Py_TYPE(other)->tp_as_number != NULL

&& Py_TYPE(other)->tp_as_number->nb_power == slot_nb_power;

picnixz · 2025-03-02T13:19:55Z

Objects/typeobject.c

-       slot_nb_power, so check before calling self.__pow__. */
+
+    /* The following code is a copy of SLOT1BINFULL, but for three arguments. */
+    PyObject* stack[3];


Suggested change

PyObject* stack[3];

PyObject *stack[3];

picnixz · 2025-03-02T13:20:32Z

Objects/typeobject.c

+                if (r != Py_NotImplemented)
+                    return r;


Suggested change

if (r != Py_NotImplemented)

return r;

if (r != Py_NotImplemented) {

return r;

}

picnixz · 2025-03-02T13:20:44Z

Objects/typeobject.c

+        if (r != Py_NotImplemented ||
+            Py_IS_TYPE(other, Py_TYPE(self)))
+            return r;


Suggested change

if (r != Py_NotImplemented ||

Py_IS_TYPE(other, Py_TYPE(self)))

return r;

if (r != Py_NotImplemented || Py_IS_TYPE(other, Py_TYPE(self))) {

return r;

}

picnixz · 2025-03-04T22:48:24Z

Sorry I didn't have time. I will leave for 10 days so I won't be able to review it. If you and Serhiy thinks it's good, then go ahead. If the PR is still not merged when I'm back then I'll try to review it.

As for whether I find it useful or not, my answer would be "yes".

pythongh-130104: Call __rpow__ in ternary pow() if necessary

85192cb

Previously it was only called in binary pow() and the binary power operator.

serhiy-storchaka requested a review from skirpichev February 18, 2025 09:54

serhiy-storchaka requested review from AA-Turner, markshannon and willingc as code owners February 18, 2025 09:54

bedevere-app bot added the awaiting core review label Feb 18, 2025

bedevere-app bot mentioned this pull request Feb 18, 2025

Inconsistent behavior of ternary pow() for Python classes and C extension types #130104

Closed

skirpichev reviewed Feb 18, 2025

View reviewed changes

Objects/typeobject.c Outdated Show resolved Hide resolved

Get rid of remnants of debugging.

b563d1c

skirpichev self-requested a review February 19, 2025 00:16

skirpichev approved these changes Feb 19, 2025

View reviewed changes

skirpichev mentioned this pull request Feb 19, 2025

gh-122193: simplify object.__rpow__() signature (no modulo) #122260

Closed

serhiy-storchaka added 2 commits February 20, 2025 12:23

"ternary" -> "three-argument"

56daac8

Merge branch 'main' into rpow

55fbb13

picnixz self-requested a review March 2, 2025 13:14

picnixz reviewed Mar 2, 2025

View reviewed changes

Merge branch 'main' into rpow

230a832

serhiy-storchaka merged commit 62ff86f into python:main Apr 16, 2025
42 checks passed

bedevere-app bot removed the awaiting core review label Apr 16, 2025

serhiy-storchaka deleted the rpow branch April 16, 2025 15:32

		@@ -9813,13 +9813,46 @@ slot_nb_power(PyObject self, PyObject other, PyObject *modulus)
		{
		if (modulus == Py_None)

Uh oh!

Conversation

serhiy-storchaka commented Feb 18, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

skirpichev commented Feb 20, 2025

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

picnixz commented Mar 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

serhiy-storchaka commented Feb 18, 2025 •

edited by github-actions bot

Loading