python / typeshed

Collection of library stubs for Python, with static types
Other
4.33k stars 1.73k forks source link

return better return type for __pow__ #4352

Closed heejaechang closed 4 years ago

heejaechang commented 4 years ago
    def __pow__(
        self, __x: int, __modulo: Optional[int] = ...
    ) -> Any: ...  # Return type can be int or float, depending on x.

pow always returns "Any" which makes more specific return type information of ** lost.

created from this issue - https://github.com/microsoft/pyright/issues/871

hauntsaninja commented 4 years ago

This comes up every now and then, everyone forgets about negative numbers :-)

srittau commented 4 years ago

Per the comment, we can't use a more specific return type. Using Union[int, float] would force users to cast or assert the return value. Using overloads wouldn't work either. Only something like python/typing#566 or literal ranges could improve the situation, but both are not supported by the current type system.

JelleZijlstra commented 4 years ago

There's also room for type checkers to special case this in specific cases. The original example in the pyright issue, for example, was 1 ** 2, and with constant operands a type checker should be able to figure out a precise type. I imagine a good proportion of usages of ** have a constant right operand.

heejaechang commented 4 years ago

@srittau can I ask why overloads won't work? thank you.

srittau commented 4 years ago

What overloads would you propose?

jakebailey commented 4 years ago

I'm finding it hard to see why Any would be better than Union[int, float]. I suppose for cases where the right side of ** is constant, the developer knows that the result type probably matches what they expect, but past that I would think they'd want to know that the result might not be an integer. A type checker would have to add all of these special cases (e.g. "if the right side is a literal non-negative integer, then int"), which seems like a fragile precedent given I could implement a library with a function that isn't on int but has a similar sort of special case that can't be represented with typing and that behavior not happen. But, I guess it is just the builtins... I just know that we take exactly what's written in the stub as the truth.

But in lieu of that return type or range handling, would it make sense to add an overload or two for the "common" right sides, like 2? I did a quick search of a bunch of projects I had checked out (django, numpy, xarray, fastai, sklearn) and the vast bulk is just ... ** 2:

     39 **1
     46 **4
     70 **32
     77 **3
    971 **2

So maybe this is "good enough":

def __pow__(self, __x: Literal[2]) -> int: ...
hauntsaninja commented 4 years ago

Thanks for doing the counting, I'd accept a PR that adds an overload for literal 2!

but past that I would think they'd want to know that the result might not be an integer

typeshed doesn't have a way to distinguish constants, so I'm not sure we can see past that. But leaving that aside, typeshed tries pretty hard to avoid false positives for any usage that's reasonably common, because our experience has typically been that users don't want to cast and assert for common usage.

Some of our general principles / previous discussion around this are outlined at https://github.com/python/typeshed/blob/master/CONTRIBUTING.md#conventions. The mypy issue linked at the relevant bit happens to use pow as their example.

jakebailey commented 4 years ago

I see, I guess I have a different philosophy in that I don't mind creating temp variables to assert my expectations, even if less convenient. I'd rather be told of a potential problem and have to manually dismiss it than to never know that the bug might be there. Returning Any seems more like the latter...

pow is certainly an annoying case, though, given I can look at 2 ** 10 and clearly know that it's an integer! But a type checker, maybe not... 🙂

I'll submit a PR (when I have time) to add the overload and see how it fares.

hauntsaninja commented 4 years ago

Two more mypy specific points: a) I believe mypy already special cases literal exponents using a plugin, b) If you prefer explicit assertions, you can use --disallow-any-expr.