bpo-41974: Remove complex.__float__, complex.__floordiv__, etc by serhiy-storchaka · Pull Request #22593 · python/cpython

serhiy-storchaka · 2020-10-08T07:00:01Z

Remove complex special methods __int__, __float__, __floordiv__,
__mod__ and __divmod__ which always raised a TypeError.

https://bugs.python.org/issue41974

Remove complex special methods __int__, __float__, __floordiv__, __mod__ and __divmod__ which always raised a TypeError.

mdickinson

LGTM, and 👍 for getting rid of these.

My only doubt is whether the phrase "real number" will make sense to those reading the error messages. It may be less confusing to leave it as "number", even though that's less precise.

serhiy-storchaka · 2020-10-08T11:19:02Z

It was noticed by Guido that error message "float() argument must be a string or a number, not 'complex'" would look confusing and he proposed to use "real number".
https://mail.python.org/archives/list/python-ideas@python.org/message/KT4JNNL6NL4L77AAQBK67GQH7U5NJ5BP/

I agreed.

mdickinson · 2020-10-08T13:20:18Z

Fair enough. The "real number" term is at least easily searchable.

vstinner · 2020-10-09T08:32:00Z

Lib/test/test_complex.py

+        with self.assertRaises(ZeroDivisionError):
+            1.0 / (0+0j)
+        with self.assertRaises(ZeroDivisionError):
+            1 / (0+0j)


It seems like these 5 tests are copy/pasted in multiple test methods. Would it be possible to use a global list and write a test using this test? Something like:

for a, b in ZERO_DIVISION: with self.subTest(a=a, b=b): with self.assertRaises(ZeroDivisionError): a / b

vstinner · 2020-10-09T08:34:09Z

Objects/complexobject.c

+        (nbr->nb_float == NULL && nbr->nb_index == NULL && !PyComplex_Check(r)))
+    {
        PyErr_Format(PyExc_TypeError,
                     "complex() first argument must be a string or a number, "


Since you change the float error, would it make sense to write "a complex number" here?

It can be a real number (int, float, Fraction, Decimal, etc). I think that requiring the first argument of the complex constructor to be a complex number would be confusing.

An "integer" number is also a real number, and a real number is also a complex number, no? I'm thinking at "real" and "complex" such as in the numbers tower. But it's just a remark, you can leave it as it is.

In mathematics it is true. But int and float are not subclasses of complex in Python.

Also, requiring an argument of the complex constructor to be a complex number would be very confusing.

In mathematics it is true. But int and float are not subclasses of complex in Python.

Oh, I didn't look at that and you're right :-)

vstinner · 2020-10-09T08:34:51Z

Lib/test/test_complex.py

                           complex(random(), random()))

-        self.assertRaises(ZeroDivisionError, complex.__truediv__, 1+1j, 0+0j)
-        self.assertRaises(OverflowError, pow, 1e200+1j, 1e200+1j)


Is this check done in another test?

Actually, no. I removed this test because it does not have relation to division (and the above test was duplicated).

Moved it into test_pow.

vstinner · 2020-10-09T11:04:46Z

Lib/test/test_complex.py

-            1 / (0+0j)
+        for a, b in ZERO_DIVISION:
+            with self.assertRaises(ZeroDivisionError):
+                a / b


Thanks, it's more readable like this!

vstinner · 2020-10-09T11:05:03Z

Lib/test/test_complex.py

        self.assertAlmostEqual(pow(1j, -1), 1/1j)
        self.assertAlmostEqual(pow(1j, 200), 1)
        self.assertRaises(ValueError, pow, 1+1j, 1+1j, 1+1j)
+        self.assertRaises(OverflowError, pow, 1e200+1j, 1e200+1j)


Alright, the best better fits into test_pow(), thanks!

vstinner

LGTM. I just have a minor remark on the NEWS entry.

vstinner · 2020-10-09T11:06:17Z

Doc/whatsnew/3.10.rst

+* Removed special methods ``__int__``, ``__float__``, ``__floordiv__``,
+  ``__mod__``, ``__divmod__``, ``__rfloordiv__``, ``__rmod__`` and
+  ``__rdivmod__`` of the :class:`complex` class.  They always raised
+  a :exc:`TypeError`.


It's unclear if it changes the behavior. Maybe explain that these operations still exist and still raise TypeError?

Sorry, I missed this comment. Could you propose new wording? Or maybe you want to make the documentation PR yourself?

It doesn't matter. Leave it as it is :-)

…nGH-22593) Remove complex special methods __int__, __float__, __floordiv__, __mod__, __divmod__, __rfloordiv__, __rmod__ and __rdivmod__ which always raised a TypeError.

the-knights-who-say-ni added the CLA signed label Oct 8, 2020

bedevere-bot added the awaiting core review label Oct 8, 2020

bpo-41974: Remove complex.__float__, complex.__floordiv__, etc

921ce08

Remove complex special methods __int__, __float__, __floordiv__, __mod__ and __divmod__ which always raised a TypeError.

serhiy-storchaka force-pushed the complex-fake-dunders branch from 9829076 to 921ce08 Compare October 8, 2020 07:34

mdickinson reviewed Oct 8, 2020

View reviewed changes

serhiy-storchaka added 2 commits October 8, 2020 14:25

Reorganize tests.

e0c41fd

Mention also __rfloordiv__, __rmod__ and __rdivmod__.

480be41

vstinner reviewed Oct 9, 2020

View reviewed changes

Address review comments.

6ac7ac9

vstinner reviewed Oct 9, 2020

View reviewed changes

vstinner approved these changes Oct 9, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Oct 9, 2020

serhiy-storchaka merged commit e2ec0b2 into python:master Oct 9, 2020

bedevere-bot removed the awaiting merge label Oct 9, 2020

serhiy-storchaka deleted the complex-fake-dunders branch October 9, 2020 11:14

Uh oh!

Conversation

serhiy-storchaka commented Oct 8, 2020 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdickinson left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Oct 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdickinson commented Oct 8, 2020

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

serhiy-storchaka commented Oct 8, 2020 •

edited by bedevere-bot

Loading

mdickinson left a comment •

edited

Loading

serhiy-storchaka commented Oct 8, 2020 •

edited

Loading