classification
Title: complex() on object with __complex__ function loses sign of zero imaginary part
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: Tom Krauss, arigo, mark.dickinson, serhiy.storchaka, steven.daprano, vstinner
Priority: normal Keywords: patch

Created on 2017-02-20 00:49 by Tom Krauss, last changed 2017-03-31 16:36 by dstufft. This issue is now closed.

Files
File name Uploaded Description Edit
complex-constructor-from-complex.patch serhiy.storchaka, 2017-02-20 18:00 review
complex-constructor-from-complex2.patch serhiy.storchaka, 2017-02-20 18:18 review
Pull Requests
URL Status Linked Edit
PR 203 merged mark.dickinson, 2017-02-20 20:09
PR 204 merged mark.dickinson, 2017-02-20 21:04
PR 205 merged mark.dickinson, 2017-02-20 21:12
PR 206 merged mark.dickinson, 2017-02-20 21:13
PR 703 larry, 2017-03-17 21:00
PR 552 closed dstufft, 2017-03-31 16:36
Messages (20)
msg288177 - (view) Author: Tom Krauss (Tom Krauss) Date: 2017-02-20 00:49
Consider the following simple class that provides a "__complex__" method.

class C(object):
  def __init__(self, x):
     self.x = x
  def __complex__(self):
    return self.x

x=C(-0j)

PYTHON 2.7.13
>>> x.x
-0j
>>> complex(x)
0j

PYTHON 3.6
>>> x.x
(-0-0j)
>>> complex(x)
(-0+0j)
msg288192 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 11:13
Confirmed on my machine. The effect of the source is to do `complex(complex(x), 0.0)` in this case, which isn't correct.
msg288196 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 11:41
Looking at the source, there's also an issue with complex subclasses here:

>>> class C(complex):
...     pass
... 
>>> complex(C(-0j))
(-0+0j)
>>> complex(-0j)
(-0-0j)
msg288205 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2017-02-20 14:04
CPython 2.7 and 3.5 have issues with the sign of zeroes even without any custom class:

>>> -(0j)       # this is -(0+0j)
(-0-0j)
>>> (-0-0j)     # but this equals to the difference between 0 and 0+0j
0j
>>> (-0.0-0j)   # this is the difference between -0.0 and 0+0j
(-0+0j)
>>> -0j
-0j             # <- on CPython 2.7
(-0-0j)         # <- on CPython 3.5

It's unclear if the signs of the two potential zeroes in a complex number have a meaning, but the C standard considers these cases for all functions in the complex number's header.
msg288207 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 14:18
Armin: that's a separate issue, and is expected behaviour.

>>> -(0j)       # this is -(0+0j)
(-0-0j)

Yes: 0j is complex(0.0, 0.0); negating negates both the real and imaginary parts.

>>> (-0-0j)     # but this equals to the difference between 0 and 0+0j
0j

This is an operation between an integer (note that the initial negation is a no-op) and a complex. Here the integer gets promoted to complex(0.0, 0.0), and we do complex(0.0, 0.0) - complex(0.0, 0.0), which gives complex(0.0, 0.0).

>>> (-0.0-0j)   # this is the difference between -0.0 and 0+0j
(-0+0j)

This is complex(-0.0, 0.0) - complex(0.0, 0.0). The real and imaginary parts are operated on separately, and in keeping with IEEE 754, the real part is evaluated as -0.0 - 0.0, which is -0.0.

>>> -0j
-0j             # <- on CPython 2.7
(-0-0j)         # <- on CPython 3.5

The Python 3 behaviour here is correct. The Python 2 behaviour is the result of an unfortunate AST optimization designed to ensure that -<sys.maxint+1> is an int rather than a long.

None of the above is a new issue.
msg288208 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 14:20
> It's unclear if the signs of the two potential zeroes in a complex number have a meaning

Yes, very much so. The signs are important in determining which side of a branch cut to use in the various cmath functions.
msg288210 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2017-02-20 14:40
Maybe I should be more explicit: what seems strange to me is that some complex numbers have a repr that, when entered in the source, produces a different result.  For example, if you want the result ``(-0-0j)`` you have to enter something different.  However, I missed the fact that calling explicitly ``complex(a, b)`` with a and b being floats always gives exactly a+bj with the correct signs.  So I retract my comments.
msg288211 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-02-20 14:43
IMHO it would be less surprising if repr(complex) would return 'complex(..., ...)': it would be possible to copy/paste and get the result value. I was bitten multiple time by the zero sign with complex numbers...
msg288212 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 14:47
Armin, Victor: please open other issues for these discussions; they're unrelated to the bug reported here. Also, see #27363, #17336, #22548, #25839.
msg288223 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-02-20 18:00
Proposed patch fixes constructing complex from complex.

But in future versions perhaps it is worth to disallow passing complex argument to two-arguments complex constructor.
msg288224 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2017-02-20 18:07
4 lines before the new "ci.real = cr.imag;", we have "cr.imag = 0.0; /* Shut up compiler warning */".  The comment is now wrong: we really need to set cr.imag to 0.0.
msg288229 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 19:27
complex-constructor-from-complex2.patch looks good to me.

What happens now? I presume we can no longer push to hg.python.org? So one of us needs to make a PR on GitHub and another review it?
msg288231 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-02-20 19:54
I pushed changes to my fork but when try to create a pull request it contains unrelated changes. Seems I do something wrong.
msg288233 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 20:10
Created https://github.com/python/cpython/pull/203, but I feel bad for having my name on the commits. :-(
msg288236 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 20:28
New changeset 112ec38c15b388fe025ccb85369a584d218b1160 by GitHub in branch 'master':
bpo-29602: fix signed zero handling in complex constructor. (#203)
https://github.com/python/cpython/commit/112ec38c15b388fe025ccb85369a584d218b1160
msg288240 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 20:57
New changeset 4ddd89780fdb823f427c743ea7326a3c958a2f4b by Mark Dickinson in branch 'bpo-29549-backport':
bpo-29602: fix signed zero handling in complex constructor
https://github.com/python/cpython/commit/4ddd89780fdb823f427c743ea7326a3c958a2f4b
msg288242 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 21:14
New changeset c0b336e0ada74b1242b9ef10c19eb87b0a21d106 by GitHub in branch '2.7':
bpo-29602: fix signed zero handling in complex constructor (#204)
https://github.com/python/cpython/commit/c0b336e0ada74b1242b9ef10c19eb87b0a21d106
msg288245 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 21:50
New changeset 0936a00fe035e3e52c30bcbc59668dc0f519ced6 by GitHub in branch '3.5':
bpo-29602: fix signed zero handling in complex constructor. (#203) (#205)
https://github.com/python/cpython/commit/0936a00fe035e3e52c30bcbc59668dc0f519ced6
msg288246 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 21:59
New changeset d9b3cdd137239a5913de2252c3ce269e35ac63d2 by GitHub in branch '3.6':
bpo-29602: fix signed zero handling in complex constructor. (#203) (#206)
https://github.com/python/cpython/commit/d9b3cdd137239a5913de2252c3ce269e35ac63d2
msg288247 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-02-20 22:04
I'm a bit puzzled about where the commit 4ddd89780fdb823f427c743ea7326a3c958a2f4b came from, but as far as I can tell the changes to 2.7, 3.5, 3.6 and master are all okay.

All fixed. Thanks for the report!
History
Date User Action Args
2017-03-31 16:36:13dstufftsetpull_requests: + pull_request889
2017-03-17 21:00:36larrysetpull_requests: + pull_request620
2017-02-20 22:04:35mark.dickinsonsetstatus: open -> closed
resolution: fixed
messages: + msg288247

stage: patch review -> resolved
2017-02-20 21:59:32mark.dickinsonsetmessages: + msg288246
2017-02-20 21:50:51mark.dickinsonsetmessages: + msg288245
2017-02-20 21:14:55mark.dickinsonsetmessages: + msg288242
2017-02-20 21:13:25mark.dickinsonsetpull_requests: + pull_request175
2017-02-20 21:12:49mark.dickinsonsetpull_requests: + pull_request174
2017-02-20 21:05:33serhiy.storchakasetpull_requests: - pull_request171
2017-02-20 21:04:01mark.dickinsonsetpull_requests: + pull_request173
2017-02-20 20:57:57mark.dickinsonsetmessages: + msg288240
2017-02-20 20:49:33serhiy.storchakasetpull_requests: + pull_request171
2017-02-20 20:28:17mark.dickinsonsetmessages: + msg288236
2017-02-20 20:10:23mark.dickinsonsetmessages: + msg288233
2017-02-20 20:09:23mark.dickinsonsetpull_requests: + pull_request169
2017-02-20 19:54:36serhiy.storchakasetmessages: + msg288231
2017-02-20 19:27:22mark.dickinsonsetmessages: + msg288229
2017-02-20 18:18:14serhiy.storchakasetfiles: + complex-constructor-from-complex2.patch
2017-02-20 18:07:30arigosetmessages: + msg288224
2017-02-20 18:00:10serhiy.storchakasetfiles: + complex-constructor-from-complex.patch
keywords: + patch
messages: + msg288223

stage: needs patch -> patch review
2017-02-20 14:47:04mark.dickinsonsetmessages: + msg288212
2017-02-20 14:43:14vstinnersetnosy: + vstinner
messages: + msg288211
2017-02-20 14:40:36arigosetmessages: + msg288210
2017-02-20 14:20:26mark.dickinsonsetmessages: + msg288208
2017-02-20 14:18:28mark.dickinsonsetmessages: + msg288207
2017-02-20 14:04:28arigosetnosy: + arigo
messages: + msg288205
2017-02-20 11:41:49serhiy.storchakasetnosy: + serhiy.storchaka
2017-02-20 11:41:15mark.dickinsonsetmessages: + msg288196
2017-02-20 11:13:16mark.dickinsonsetassignee: mark.dickinson
2017-02-20 11:13:02mark.dickinsonsetstage: needs patch
messages: + msg288192
components: + Interpreter Core
versions: + Python 3.5, Python 3.7
2017-02-20 09:37:16xiang.zhangsetnosy: + mark.dickinson
2017-02-20 01:28:52steven.dapranosetnosy: + steven.daprano
2017-02-20 00:49:52Tom Krausscreate