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

Type Hints #3

Closed
mentalisttraceur opened this issue Oct 7, 2023 · 6 comments
Closed

Type Hints #3

mentalisttraceur opened this issue Oct 7, 2023 · 6 comments

Comments

@mentalisttraceur
Copy link
Owner

mentalisttraceur commented Oct 7, 2023

I have an idea, but I don't know if nested overloads can combine like this, and might not have time to check for a while... would really appreciate help looking into it (and if it doesn't work, help with figuring out how to make it work or what the next best thing is):

P = ParamSpec('P')
R1 = TypeVar('R1')
R2 = TypeVar('R2')
A1 = Awaitable[R1]
A2 = Awaitable[R2]

@overload
class composable(Callable[P, R1]):
    @overload
    def __or__(self, other: Callable[[R1], R2]) -> composable[P, R2]:
        ...
    @overload
    def __or__(self, other: Callable[[R1], A2]) -> composable[P, A2]:
        ...
@overload
class composable(Callable[P, A1]):
    @overload
    def __or__(self, other: Callable[[R1], R2]) -> composable[P, A2]:
        ...
    @overload
    def __or__(self, other: Callable[[R1], A2]) -> composable[P, A2]:
        ...
@overload
class composable(Callable[[R1], R2]):
    @overload
    def __ror__(self, other: Callable[P, R1]) -> composable[P, R2]:
        ...
    @overload
    def __ror__(self, other: Callable[P, A1]) -> composable[P, A2]:
        ...
@overload
class composable(Callable[[R1], A2]):
    @overload
    def __ror__(self, other: Callable[P, R1]) -> composable[P, A2]:
        ...
    @overload
    def __ror__(self, other: Callable[P, A1]) -> composable[P, A2]:
        ...

@ruancomelli, you did some work figuring this out in pytoolz/toolz#523 (comment) - any thoughts?

@mentalisttraceur
Copy link
Owner Author

mentalisttraceur commented Oct 7, 2023

Past experience with adding type hints for compose suggests that Pyre and Pyright are more capable than Mypy, and I think I'd be fine with doing something that works in some type checkers but not others - after all, that's part of how we get Python's type ecosystem to become good enough for what the community needs (although more ideal would be something that progressively enhances / gracefully degrades).

@mentalisttraceur
Copy link
Owner Author

Alright, so after fiddling w/ it a couple hours (mypy and pyright), the results are discouraging enough that I am tempted to just set this aside indefinitely. Here's as far as I got (started with the above sketch and then iteratively fixed errors):

from collections.abc import Callable
from typing import Awaitable, Generic, ParamSpec, TypeVar, overload


P = ParamSpec('P')
R1 = TypeVar('R1')
R2 = TypeVar('R2')

@overload
class composable(Generic[R1, R2]):
    __wrapped__: Callable[[R1], Awaitable[R2]]
    def __init__(self, function: Callable[[R1], Awaitable[R2]]):
        ...
    def __call__(self, argument: R1) -> Awaitable[R2]:
        ...
    @overload
    def __ror__(self, other: Callable[P, Awaitable[R1]]) -> 'composable[R1, Awaitable[R2]]':
        ...
    @overload
    def __ror__(self, other: Callable[P, R1]) -> 'composable[P, Awaitable[R2]]':
        ...
@overload
class composable(Generic[R1, R2]):
    __wrapped__: Callable[[R1], R2]
    def __init__(self, function: Callable[[R1], R2]):
        ...
    def __call__(self, argument: R1) -> R2:
        ...
    @overload
    def __ror__(self, other: Callable[P, Awaitable[R1]]) -> 'composable[R1, Awaitable[R2]]':
        ...
    @overload
    def __ror__(self, other: Callable[P, R1]) -> 'composable[P, R2]':
        ...
@overload
class composable(Generic[P, R1]):
    __wrapped__: Callable[P, Awaitable[R1]]
    def __init__(self, function: Callable[P, Awaitable[R1]]):
        ...
    def __call__(self, *args: P.args, **kwargs: P.kwargs) -> Awaitable[R1]:
        ...
    @overload
    def __or__(self, other: Callable[[R1], Awaitable[R2]]) -> 'composable[P, Awaitable[R2]]':
        ...
    @overload
    def __or__(self, other: Callable[[R1], R2]) -> 'composable[P, Awaitable[R2]]':
        ...
@overload
class composable(Generic[P, R1]):
    __wrapped__: Callable[P, R1]
    def __init__(self, function: Callable[P, R1]):
        ...
    def __call__(self, *args: P.args, **kwargs: P.kwargs) -> R1:
        ...
    @overload
    def __or__(self, other: Callable[[R1], Awaitable[R2]]) -> 'composable[P, Awaitable[R2]]':
        ...
    @overload
    def __or__(self, other: Callable[[R1], R2]) -> 'composable[P, R2]':
        ...

Couple notes:

  1. I did this as a separate hints file. So compose_operator.py -> compose_operator/__init__.py and put the type hints in compose_operator/__init__.pyi. I'm not sure if this is actually true, but to me it feels like it simplifies things - I know just the .pyi file is used, so I don't have to worry about any typing information in the .py file.

  2. Here's a basic minimal test file I was trying it with:

    from compose_operator import *
    
    @composable
    def f(x: int, y: float) -> float:
        return x + y
    
    @composable
    def g(x: float) -> str:
        return str(x)
    
    @composable
    async def a(x: float) -> int:
        return int(x) // 2
    
    (f | int | g)(1, 2.0)
    
    async def b(x: int) -> str:
        return await ((f | a | float | g)(x, 3.0)) 

So funny thing. MyPy trips all over it with 16 errors and Pyright only reports one error, but they have that one error in common: they assume that | is always a type union.

In retrospect, I should've expected this. Of course the simplest thing for static type checkers to do is to assume, statically, that when they see a type and a |, it's a type union.

I don't blame the static type checkers here, since the general solution would require symbolically executing arbitrary python code to analyze whether or not it's overloading the | operator in a way that overrides type union behavior. That's halting-problem level stuff I'm pretty sure. So I'm not sure that this is going to be ever fixable, unless ecosystem for type hints starts providing an explicit way to statically indicate that a thing is overloading the | for type unions.

@mentalisttraceur
Copy link
Owner Author

TL;DR:

The choice of | for the composition operator turns out to be uniquely unsuited for static type checking.

Overloading that works fine at runtime, but static type checkers currently assume that | with types is always a type union, and the typing stuff doesn't provide any way to statically say through the type system that it means anything else.

@mentalisttraceur
Copy link
Owner Author

mentalisttraceur commented Oct 7, 2023

Another problem with what I've got so far is that even though it almost works in Pyright when types are out of the picture, Pyright fails to recognize composable as a callable, and so it fails as soon as we're chaining two or more |. And MyPy still has like 15 errors that I just don't want to even look at anymore.

None of this was problem in compose. The obvious brute force way Just Works for those for any set of hard-coded arities (although, admittedly, over there I lose type hinting for the other stuff like the .functions attribute, and maybe the same solutions would cover both).

@mentalisttraceur
Copy link
Owner Author

I tried this earlier, but it actually just gave false positive successes for all compositions where types didn't match.

from collections.abc import Callable
from typing import Awaitable, ParamSpec, TypeVar, overload


P = ParamSpec('P')
R1 = TypeVar('R1')
R2 = TypeVar('R2')

@overload
class composable(Callable[[R1], Awaitable[R2]]):
    __wrapped__: Callable[[R1], Awaitable[R2]]
    def __init__(self, function: Callable[[R1], Awaitable[R2]]):
        ...
    @overload
    def __ror__(self, other: Callable[P, Awaitable[R1]]) -> 'composable[P, Awaitable[R2]]':
        ...
    @overload
    def __ror__(self, other: Callable[P, R1]) -> 'composable[P, Awaitable[R2]]':
        ...
@overload
class composable(Callable[[R1], R2]):
    __wrapped__: Callable[[R1], R2]
    def __init__(self, function: Callable[[R1], R2]):
        ...
    @overload
    def __ror__(self, other: Callable[P, Awaitable[R1]]) -> 'composable[P, Awaitable[R2]]':
        ...
    @overload
    def __ror__(self, other: Callable[P, R1]) -> 'composable[P, R2]':
        ...
@overload
class composable(Callable[P, Awaitable[R1]]):
    __wrapped__: Callable[P, Awaitable[R1]]
    def __init__(self, function: Callable[P, Awaitable[R1]]):
        ...
    @overload
    def __or__(self, other: Callable[[R1], Awaitable[R2]]) -> 'composable[P, Awaitable[R2]]':
        ...
    @overload
    def __or__(self, other: Callable[[R1], R2]) -> 'composable[P, Awaitable[R2]]':
        ...
@overload
class composable(Callable[P, R1]):
    __wrapped__: Callable[P, R1]
    def __init__(self, function: Callable[P, R1]):
        ...
    @overload
    def __or__(self, other: Callable[[R1], Awaitable[R2]]) -> 'composable[P, Awaitable[R2]]':
        ...
    @overload
    def __or__(self, other: Callable[[R1], R2]) -> 'composable[P, R2]':
        ...

@mentalisttraceur
Copy link
Owner Author

I'm closing this issue to indicate that I personally am no longer actively working on it, but I still welcome comments and help from others.

If someone brings a working set of type hints I'll gladly merge it, and I'd gladly add a second operator to entirely bypass the type union ambiguity. I'll even accept working type-hints for just reduced sets of cases (e.g. no async). And I'm happy to read/discuss different attempts/ideas in-depth.

I'll cut v1.0.0 without static type hints - those can be a future feature release.

In the meantime: in most code-bases where I'd want static type checking, I'd use compose - it has good type hints (on Python 3.10 or later).

To me, the operator-overload approach has always been the kind of cheeky fun convenience that I'd use in fast prototyping, one-off scripts, and interactive REPL use - all uses where static typing is overhead more than help.

Operator overload like this is far enough from the standard mainstream of the language that I wouldn't use it in a code-base that I thought might be maintained by some random junior- or mid-level Python dev in five years, because the ecosystem might move on in ways that break it further, and that dev is probably reading while expecting/assuming idiomatic code, and might more slowed than helped by needing to adjust their interpretation of |.

I see | for composition as in the same class as the placeholder package - excellent ergonomics if you're hand-typing a bunch of functional code, but not the most optimal for long-term understanding and maintenance by devs who are familiar with Python rather than this specific almost-syntactic-extension of it.

So I don't think it's actually a major loss. I just had hoped that it would be doable enough that I might as well add it, but it no longer seems that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant