Skip to content
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

pow(complex(…), <value>, <value>) and pow(float(…), <value>, <value>) should produce type errors #6303

Closed
posita opened this issue Nov 15, 2021 · 13 comments
Labels
stubs: false negative Type checkers do not report an error, but should

Comments

@posita
Copy link
Contributor

posita commented Nov 15, 2021

complex's versions of __pow__ and __rpow__ invite the third modulo argument. That means that the following validates, when it shouldn't:

This applies to floats as well.

pow(complex(1), 2, 2)  # no type error?
pow(float(1), 2, 2)  # still no type error?

These are guaranteed to fail at runtime, even if the first arguments are losslessly convertible to ints:

>>> pow(complex(1), 2, 2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: complex modulo
>>> pow(float(1), 2, 2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: pow() 3rd argument not allowed unless all arguments are integers

I can't find an issue tracking this, and I don't think this is fixed by #6287.


As an aside, this "works" (in the sense it properly generates some error, but not what I would expect):

from numbers import Complex
my_complex: Complex = complex(1)  # type: ignore [assignment]  # see 3186
pow(my_complex, 2, 2)  # No overload variant of "pow" matches argument types "Complex", "int", "int"

That second error might just be some variation on python/mypy#3186, however?

@AlexWaygood
Copy link
Member

I don't think the fix can be as simple as simply removing the third argument of float.__pow__/complex.__pow__, given that the following all succeed at runtime without error:

>>> pow(1.0, 2, None)
1.0
>>> 1.0.__pow__(2, None)
1.0
>>> pow(complex(1), 2, None)
(1+0j)
>>> complex(1).__pow__(3, None)
(1+0j)

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 15, 2021

Perhaps if we changed builtins._M (line 1283) from

from typing import TypeVar
_M = TypeVar("_M", contravariant=True)

to

from typing import TypeVar, SupportsIndex
_M = TypeVar("_M", contravariant=True, bound=SuppprtsIndex)

?

@posita
Copy link
Contributor Author

posita commented Nov 15, 2021

And, to be fair, this works just fine at runtime:

>>> pow(complex(1), 2, None)
(1+0j)

In the bound proposal, is the intention to use SupportsIndex as a proxy for numbers.Integral-like things? (Kind of like how math.trunc accepts SupportsFloat?)

@srittau srittau added the stubs: false negative Type checkers do not report an error, but should label Nov 15, 2021
@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 15, 2021

In the bound proposal, is the intention to use SupportsIndex as a proxy for numbers.Integral-like things? (Kind of like how math.trunc accepts SupportsFloat?)

Yeah, sort of. I think Mypy currently thinks pow(2.5, 3.9, 6) is okay because of the final overload of pow in line 1309, where we've told it that pow can be used with three arguments as long as the type passed in for argument 1 is annotated as having a __pow__ method that accepts three arguments. __float__ and __complex__ both are, and that's correct, because of course the None argument can be passed in explicitly at runtime! But the _M TypeVar used to parameterize the _SupportsPow3 protocol isn't restrictive enough.

@AlexWaygood
Copy link
Member

Gotta go but some WIP experiments here https://mypy-play.net/?mypy=latest&python=3.10&flags=show-error-codes%2Cstrict&gist=b079cabb8620757ef16301a2dc12a352

@AlexWaygood
Copy link
Member

Hmm, it's not the best error message, but this does seem to work: https://mypy-play.net/?mypy=latest&python=3.10&flags=show-error-codes%2Cstrict&gist=4e463e07a3a48610e140a4c98c18b0ca

It involves changing the 5th overload from

def pow(base: float, exp: float, mod: None = ...) -> Any: ...

to

def pow(base: complex, exp: complex, mod: None = ...) -> Any: ...

as well as the change to the _M TypeVar.

But, I haven't yet tested it with classes outside builtins.

@posita
Copy link
Contributor Author

posita commented Nov 15, 2021

That's some expert level deep typing voodoo.

I just thought of another wrinkle. Mypy treats floats as equivalent to ints for many (all?) purposes. That may make guarding against that particular case problematic.

I also just noticed that Decimal is weird (but nobody cares about Decimal, so oh well, I guess):

>>> pow(Decimal("1.0"), Decimal("1.0"), Decimal("1.0"))  # works
Decimal('0')

@AlexWaygood
Copy link
Member

Weird false-positive error in this implementation if you try to do pow(Fraction, complex) (though honestly, why would you??)

https://mypy-play.net/?mypy=latest&python=3.10&flags=show-error-codes%2Cstrict&gist=5b89b8868672e4cac1b4f6e2ead4422e

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 15, 2021

This is a better solution, I think -- introduce a third protocol to deal with classes that accept "three-argument pow but only if the third argument is None", rather than mucking around with binding the _M TypeVar. It even means that we can deal with the Decimal weirdness!

from typing import TypeVar, Protocol, Any, Literal, NoReturn, overload

_E = TypeVar("_E", contravariant=True)
_M = TypeVar("_M", contravariant=True)
_T_co = TypeVar("_T_co", covariant=True)

class _SupportsPow2(Protocol[_E, _T_co]):
    def __pow__(self, other: _E) -> _T_co: ...
    
class _SupportsPow3NoneOnly(Protocol[_E, _T_co]):
    def __pow__(self, other: _E, modulo: None = ...) -> _T_co: ...

class _SupportsPow3(Protocol[_E, _M, _T_co]):
    def __pow__(self, other: _E, modulo: _M = ...) -> _T_co: ...


@overload
def pow(base: int, exp: int, mod: Literal[0]) -> NoReturn: ...
# int base & positive-int exp -> int; int base & negative-int exp -> float
# return type must be Any as `int | float` causes too many false-positive errors
@overload
def pow(base: int, exp: int, mod: None = ...) -> Any: ...
@overload
def pow(base: int, exp: int, mod: int) -> int: ...
@overload
def pow(base: float, exp: int, mod: None = ...) -> float: ...
# float base & float exp could return float or complex
# return type must be Any as `float | complex` causes too many false-positive errors
@overload
def pow(base: complex, exp: complex, mod: None = ...) -> Any: ...
@overload
def pow(base: _SupportsPow2[_E, _T_co], exp: _E, mod: None = ...) -> _T_co: ...
@overload
def pow(base: _SupportsPow3NoneOnly[_E, _T_co], exp: _E, mod: None = ...) -> _T_co: ...
@overload
def pow(base: _SupportsPow3[_E, _M, _T_co], exp: _E, mod: _M = ...) -> _T_co: ...

There's still a false positive with Fraction ** complex though, and I can't work out why.

https://mypy-play.net/?mypy=latest&python=3.10&flags=show-error-codes%2Cstrict&gist=36736ab9a3edb91e8e890e5e373ec639

@posita
Copy link
Contributor Author

posita commented Nov 15, 2021

I definitely need to up my TypeVar game. The thing that set me on this path was trying to type a helpful pow wrapper that would work with numerary (which has much less sophisticated SupportsComplexPow and SupportsIntegralPow protocols more akin to numbers.Complex and numbers.Integral).

@AlexWaygood
Copy link
Member

I don't think this actually solves your original problem, but, er, here's a PR fixing some other problems with pow! #6325

@AlexWaygood
Copy link
Member

AlexWaygood commented Dec 29, 2021

@posita, I think this should maybe be refiled as a mypy issue now, since we now get error messages in the right places, it's just not the right kind of error messages. That feels like more of a mypy issue rather than a typeshed issue -- WDYT? 🙂

@AlexWaygood
Copy link
Member

@posita, I think this should maybe be refiled as a mypy issue now, since we now get error messages in the right places, it's just not the right kind of error messages. That feels like more of a mypy issue rather than a typeshed issue -- WDYT? 🙂

Closing as per my comments here

@AlexWaygood AlexWaygood closed this as not planned Won't fix, can't repro, duplicate, stale Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: false negative Type checkers do not report an error, but should
Projects
None yet
Development

No branches or pull requests

3 participants