gh-78857: Always return a 2-tuple from divmod() and PyNumber_Divmod(). #9301

serhiy-storchaka · 2018-09-14T10:23:59Z

https://bugs.python.org/issue34676

Issue: Guarantee that divmod() and PyNumber_Divmod() return a 2-tuple #78857

pganssle · 2018-09-14T16:36:41Z

Misc/NEWS.d/next/Core and Builtins/2018-09-14-13-16-27.bpo-34676.A7yio5.rst

@@ -0,0 +1,2 @@
+It is now guarantied that :func:`divmod` and :c:func`PyNumber_Divmod` always


s/guarantied/guaranteed/

pganssle · 2018-09-17T13:29:44Z

@serhiy-storchaka Seems to be broken because magic mocks just return a magic mock. That could possibly be considered a bug in unittest.mock.MagicMock.

eric-wieser · 2019-04-14T07:00:01Z

Objects/abstract.c

+    }
+    if (!PyTuple_Check(result)) {
+        orig_result = result;
+        result = PySequence_Tuple(result);


You could Py_DECREF(orig_result) right here, and then you could eliminate the 3 Py_(X)DECREF(orig_result)s below

Then there will be an undefined behavior in orig_result != NULL.

I missed the connection between Py_DECREF and free here, good point.

Setting a single boolean in this branch would eliminate the need for the orig_result != NULL, and would arguably be clearer - but I suppose that's bike-shedding

I now remember why I used this way. Keeping the reference to orig_result allows to use it in the error/warning message. The current version does not use it, but I was going to make a separate PR for more detailed error messages for all operations.

eric-wieser · 2019-04-14T07:20:50Z

Lib/test/test_builtin.py

+            self.assertEqual(divmod([], fake_rdivmod([2, 3])), (2, 3))
+        self.assertRaises(TypeError, divmod, [], fake_rdivmod(42))
+        self.assertRaises(TypeError, divmod, [], fake_rdivmod(()))
+        self.assertRaises(TypeError, divmod, [], fake_rdivmod((1, 2, 3)))


Might be good to add a namedtuple example here, to state explicitly that tuple subclasses are intended to be supported.

I hesitate. Should we allow tuple subclasses or require exact tuples? Most converting special methods (__int__, __index__, __float__, __complex__) are required to return an instance of exact type. From C side, if use the C API for unpacking the tuple, there is no difference between tuples and tuple subclasses. But there mey be difference at Python level if the tuple subclass overrides some methods like __add__, __iter__, __len__, __getitem__.

For now, I prefer to not promise the namedtuple support.

pganssle · 2019-04-16T17:42:27Z

Modules/_datetimemodule.c

@@ -1836,7 +1813,7 @@ microseconds_to_delta_ex(PyObject *pyus, PyTypeObject *type)
    PyObject *num = NULL;
    PyObject *result = NULL;

-    tuple = checked_divmod(pyus, us_per_second);
+    tuple = PyNumber_Divmod(pyus, us_per_second);


I haven't looked closely at any of the other stuff, but +1 on the _datetimemodule.c stuff. It is very nice that it is now possible to make this simplification.

Doc/whatsnew/3.10.rst

Co-authored-by: Eric Wieser <wieser.eric@gmail.com>

rhettinger · 2022-05-01T06:58:55Z

I don't think this should be applied. There isn't a known user issue being solved. Also, this changes a very old API so it might break some deployed code. Also, IIRC it created some issue with magic mocks.

serhiy-storchaka · 2022-05-01T08:47:36Z

The problem is that virtually any use of PyNumber_Divmod() requires a boilerplate code to check that it is a tuple and its size is 2. User code often omit that check.

Examples:
https://github.com/lycantropos/symba/blob/e5be13c43d9da2c0920ccb351b934ae2953f92d4/src/symba.c
https://github.com/tmroeder/pyScore/blob/b460b5912048d22d83c0375d1b0e248f6883a8c2/src/crat/cratmodule.c

There were similar issues in Python core code, until they were fixed.

Alternatively we can provide a new public API for calling PyNumber_Divmod() and unpacking the result. It would not help the old code which uses PyNumber_Divmod() followed by PyTuple_GET_ITEM(), but it may make writing the new correct code less painful.

As for mocks, I think that it does not create some issue with magic mocks, but exposes some existing issues with magic mocks.

rhettinger · 2022-05-03T06:27:00Z

-0

I would just let it be. ISTM that API restrictions are almost always more hassle than they are worth.

iritkatriel

This has merge conflicts now.

iritkatriel · 2020-09-25T10:23:50Z

Doc/whatsnew/3.9.rst

@@ -764,6 +764,11 @@ Changes in the Python API
  environment variable when the :option:`-E` or :option:`-I` command line
  options are being used.

+* :func:`divmod` and :c:func:`PyNumber_Divmod` will check the result of special
+  methods ``__divmod__`` and ``__rdivmod__`` and emit a deprecation warning if
+  it is not a tuple or raise an error if it's size is not 2.


it's size --> its size

bedevere-bot · 2022-11-25T10:59:17Z

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

github-actions · 2024-12-15T00:12:20Z

This PR is stale because it has been open for 30 days with no activity.

github-actions · 2025-04-13T06:03:17Z

This PR is stale because it has been open for 30 days with no activity.

bpo-34676: Always return a 2-tuple from divmod() and PyNumber_Divmod().

4a699eb

serhiy-storchaka added the type-feature A feature request or enhancement label Sep 14, 2018

the-knights-who-say-ni added the CLA signed label Sep 14, 2018

bedevere-bot added the awaiting merge label Sep 14, 2018

pganssle reviewed Sep 14, 2018

View reviewed changes

serhiy-storchaka added 2 commits September 17, 2018 11:14

Convert iterables to tuple.

2142008

Merge branch 'master' into divmod

70132f8

Fix tests and mock.

eb6947e

serhiy-storchaka requested review from rhettinger and skrah as code owners September 17, 2018 19:42

Fix a typo.

c011052

skrah removed their request for review September 17, 2018 20:17

Fix RST mark up.

5ec28de

eric-wieser reviewed Apr 14, 2019

View reviewed changes

serhiy-storchaka added 2 commits April 15, 2019 20:25

Merge branch 'master' into divmod

2d5c3cc

Simplify _datetimemodule.c.

f8e551f

pganssle reviewed Apr 16, 2019

View reviewed changes

serhiy-storchaka added 2 commits April 1, 2020 16:57

Merge branch 'master' into divmod

159a37b

Update version.

60adab4

serhiy-storchaka requested a review from abalkin as a code owner April 1, 2020 15:55

Merge branch 'master' into divmod

eb0be0e

eric-wieser reviewed Oct 24, 2020

View reviewed changes

Doc/whatsnew/3.10.rst Outdated Show resolved Hide resolved

Update Doc/whatsnew/3.10.rst

148ac2d

Co-authored-by: Eric Wieser <wieser.eric@gmail.com>

rhettinger removed their request for review May 3, 2022 06:29

ezio-melotti removed the CLA signed label Jul 13, 2022

iritkatriel requested changes Nov 25, 2022

View reviewed changes

bedevere-bot removed the awaiting merge label Nov 25, 2022

bedevere-bot added the awaiting changes label Nov 25, 2022

serhiy-storchaka mentioned this pull request Apr 10, 2022

Guarantee that divmod() and PyNumber_Divmod() return a 2-tuple #78857

Open

github-actions bot added the stale Stale PR or inactive for long period of time. label Dec 15, 2024

picnixz changed the title ~~bpo-34676: Always return a 2-tuple from divmod() and PyNumber_Divmod().~~ gh-78857: Always return a 2-tuple from divmod() and PyNumber_Divmod(). Dec 15, 2024

github-actions bot removed the stale Stale PR or inactive for long period of time. label Feb 27, 2025

github-actions bot added the stale Stale PR or inactive for long period of time. label Apr 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-78857: Always return a 2-tuple from divmod() and PyNumber_Divmod(). #9301

gh-78857: Always return a 2-tuple from divmod() and PyNumber_Divmod(). #9301

serhiy-storchaka commented Sep 14, 2018 •

edited by bedevere-app bot

Loading

pganssle Sep 14, 2018

pganssle commented Sep 17, 2018

eric-wieser Apr 14, 2019

serhiy-storchaka Apr 14, 2019

eric-wieser Apr 14, 2019

serhiy-storchaka Apr 14, 2019

eric-wieser Apr 14, 2019

serhiy-storchaka Apr 14, 2019

pganssle Apr 16, 2019

rhettinger commented May 1, 2022

serhiy-storchaka commented May 1, 2022

rhettinger commented May 3, 2022

iritkatriel left a comment

iritkatriel Sep 25, 2020

bedevere-bot commented Nov 25, 2022

github-actions bot commented Dec 15, 2024

github-actions bot commented Apr 13, 2025

		@@ -0,0 +1,2 @@
		It is now guarantied that :func:`divmod` and :c:func`PyNumber_Divmod` always

gh-78857: Always return a 2-tuple from divmod() and PyNumber_Divmod(). #9301

Are you sure you want to change the base?

gh-78857: Always return a 2-tuple from divmod() and PyNumber_Divmod(). #9301

Conversation

serhiy-storchaka commented Sep 14, 2018 • edited by bedevere-app bot Loading

Choose a reason for hiding this comment

pganssle commented Sep 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhettinger commented May 1, 2022

serhiy-storchaka commented May 1, 2022

rhettinger commented May 3, 2022

iritkatriel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bedevere-bot commented Nov 25, 2022

github-actions bot commented Dec 15, 2024

github-actions bot commented Apr 13, 2025

serhiy-storchaka commented Sep 14, 2018 •

edited by bedevere-app bot

Loading