Skip to content

Types for complex.__new__ should be any numeric type #8440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
smlerman opened this issue Jul 29, 2022 · 20 comments
Closed

Types for complex.__new__ should be any numeric type #8440

smlerman opened this issue Jul 29, 2022 · 20 comments
Labels
stubs: false positive Type checkers report false errors

Comments

@smlerman
Copy link
Contributor

The types for the arguments to complex.__new__ should be any numeric type, not just float.

c1 = complex(1, 2)
c2 = complex(1, c1)

This code is valid, but the second line reports an error that complex is incompatible with float.

def __new__(cls: type[Self], real: float = ..., imag: float = ...) -> Self: ...

@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 29, 2022

The docs for complex state:

Each argument may be any numeric type (including complex).

So your analysis seems correct. PR welcome!

@AlexWaygood AlexWaygood added stubs: false positive Type checkers report false errors size-small labels Jul 29, 2022
@smlerman
Copy link
Contributor Author

I tried changing float to numbers.Number, but I got errors that int, float, and complex are all incompatible. Is that expected? If so, changing it to float | complex seems to work correctly.

@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 29, 2022

I tried changing float to numbers.Number, but I got errors that int, float, and complex are all incompatible. Is that expected? If so, changing it to float | complex seems to work correctly.

Yeah, that's expected (see the resources I laid out here for why numbers.Number isn't much use when it comes to static typing).

For the first argument, the docs state:

For a general Python object x, complex(x) delegates to x.__complex__(). If __complex__() is not defined then it falls back to __float__(). If __float__() is not defined then it falls back to __index__().

That means that the first argument should be complex | SupportsComplex | SupportsFloat | SupportsIndex.

For the second argument, it seems complex or anything with a __float__ method will do, so the argument should be complex | SupportsFloat:

>>> class Foo:
...   def __index__(self):
...     return 5
...     
>>> complex(1, Foo())
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: complex() second argument must be a number, not 'Foo'
>>> class Foo:
...   def __complex__(self):
...     return 5
...     
>>> complex(1, Foo())
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: complex() second argument must be a number, not 'Foo'
>>> class Foo:
...   def __float__(self):
...     return 5
...     
>>> complex(1, Foo())
(1+5j)

@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 29, 2022

SupportsComplex and SupportsFloat are both importable from typing; SupportsIndex is importable from typing_extensions (can't import it from typing as typeshed supports Python 3.7 and SupportsIndex was only added in 3.8).

@AlexWaygood
Copy link
Member

For the first argument, the docs state:

For a general Python object x, complex(x) delegates to x.__complex__(). If __complex__() is not defined then it falls back to __float__(). If __float__() is not defined then it falls back to __index__().

That means that the first argument should be complex | SupportsComplex | SupportsFloat | SupportsIndex.

Actually, looks like that's only the case for when you only provide a single argument to complex(). If you provide two arguments, then the first argument should also be complex | SupportsFloat.

@AlexWaygood
Copy link
Member

This is more complicated than I realised; I can prepare the PR if you like :)

@smlerman
Copy link
Contributor Author

I was going to ask what Python version you were using, because I was getting different results with your code, then I saw your comment about using 3.7.

There's already a one-parameter overload declared, which has str | SupportsComplex | SupportsIndex | complex (should maybe add SupportsFloat; I can test it out), so I think I'd only have to fix the two-parameter overload.

@smlerman
Copy link
Contributor Author

complex(1, FooSupportsIndex()) only works in 3.8+. Is it worth putting in a version check with different declarations for <3.8 and >=3.8?

@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 29, 2022

complex(1, FooSupportsIndex()) only works in 3.8+. Is it worth putting in a version check with different declarations for <3.8 and >=3.8?

Yes, absolutely! And, good catch!

@smlerman
Copy link
Contributor Author

smlerman commented Jul 29, 2022

python_typeshed_8440.py.txt
Attached a script for checking everything

@smlerman
Copy link
Contributor Author

For some bizarre reason, Python gives TypeError: complex() second argument must be a number, not 'FooComplex' for a SupportsComplex object, even though a complex works fine. This is on all versions up to 3.10.

@AlexWaygood
Copy link
Member

For some bizarre reason, Python gives TypeError: complex() second argument must be a number, not 'FooComplex' for a SupportsComplex object, even though a complex works fine. This is on all versions up to 3.10.

Some aspects of Python's data model are a little rough around the edges :)

@smlerman
Copy link
Contributor Author

I assume the declaration here should match Python's actual behavior, and not what it probably should be?

@AlexWaygood
Copy link
Member

I assume the declaration here should match Python's actual behavior, and not what it probably should be?

Yes, absolutely. "What the behaviour should be" is the question you ask over at the CPython repo, not at typeshed :)

smlerman added a commit to smlerman/python-typeshed that referenced this issue Jul 29, 2022
For two-parameter overload:
 - Change first parameter to complex | SupportsComplex | SupportsFloat
 - Change second parameter to complex | SupportsFloat (Python doesn't allow SupportsComplex for the second parameter, though it isn't clear why)
 - Add SupportsIndex to both parameters for Python >= 3.8
For one-parameter overload:
 - Remove SupportsIndex for Python < 3.8
@smlerman
Copy link
Contributor Author

Take a look at smlerman@bc74659 and let me know if that looks right.

@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 30, 2022

Take a look at smlerman@bc74659 and let me know if that looks right.

That looks great! The only thing to change is that SupportsFloat should be added to the union for the one-argument overload in both the >= (3, 8) branch and the < (3, 8) branch.

For the two-argument overload, it is utterly bizarre that complex accepts SupportsComplex for the first argument, but not the second, but that does indeed seem to be the case!

@JelleZijlstra
Copy link
Member

The C code (for 3.12) is here: https://github.com/python/cpython/blob/cd26595232ac1b5061460d5949d5204c90287c1c/Objects/complexobject.c#L898. It indeed uses __complex__ only for the real argument, not the imaginary one.

smlerman added a commit to smlerman/python-typeshed that referenced this issue Aug 2, 2022
Add `SupportsFloat` to one-parameter overload, per python#8440 (comment)
@smlerman
Copy link
Contributor Author

smlerman commented Aug 2, 2022

Just added smlerman@070a113

@AlexWaygood
Copy link
Member

Looks great! Please feel free to submit a PR with the fix :)

@AlexWaygood
Copy link
Member

Fixed by #8473. Thanks @smlerman!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: false positive Type checkers report false errors
Projects
None yet
Development

No branches or pull requests

3 participants