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 hint error of wrapped function #122

Closed
medihack opened this issue Nov 15, 2023 · 7 comments
Closed

Type hint error of wrapped function #122

medihack opened this issue Nov 15, 2023 · 7 comments

Comments

@medihack
Copy link

medihack commented Nov 15, 2023

According to #102 Pebble does support type hints. But if I call a wrapped function as below then pyright tells me 'Object of type "ProcessFuture" is not callable'.. It also tells me that the wrapped function if of type (function) foo: ProcessFuture | _Wrapped[(*args: Any, **kwargs: Any), Any, (*args: Unknown, **kwargs: Unknown), ProcessFuture]. Not sure what the problem with the type checker here is or if it can be improved on the Pebble side. I am using v5.0.3.

from pebble import concurrent

@concurrent.process(timeout=10)
def foo():
    pass

x = foo() # <- type error here
@noxdafox
Copy link
Owner

Hello,

it surely is complicated to infer the type in this case as decorators are hiding lots of details to the linters.

In this case, x is definitely of type ProcessFuture as this is what the decorated function will return. Yet, I will add type hints to those functions to see if it helps anyhow the linter.

noxdafox added a commit that referenced this issue Nov 21, 2023
noxdafox added a commit that referenced this issue Nov 21, 2023
noxdafox added a commit that referenced this issue Nov 21, 2023
@noxdafox
Copy link
Owner

Release 5.0.4 should address this issue. Please re-open if problems persist.

@medihack
Copy link
Author

medihack commented Nov 23, 2023

@noxdafox Now it's even worse than before (at least with pyright) ;-) Before the change, I could do an assert callable(self.foo), and then x was a ProcessFuture. Now it is always the return type of the foo function itself and never a ProcessFuture. But yes, I am not sure if it is even possible to type it correctly in Python.

@noxdafox noxdafox reopened this Nov 26, 2023
@noxdafox
Copy link
Owner

noxdafox commented Nov 26, 2023

These things are half following the type hints and half guessing themselves.

I will try a bit harder.

How can I scan pyright on a single file? I tried on the exact code snippet of yours and it didn't report any issue.

@medihack
Copy link
Author

You can call pyright filename.py or install the pylance extension when using VS Code which uses pyright internally for live checking inside the editor.

Here is a simple example and the error message:

from pebble import concurrent


@concurrent.process(daemon=True)
def foo(x: int) -> str:
    return str(x)


future = foo(42)

future.result() # Pyright error: Cannot access member "result" for type "str", Member "result" is unknown

I would expect future to be a ProcessFuture, which it was in the previous version, but only after type narrowing with assert callable(foo). Also, the function parameters of @concurrent.process, e.g., daemon don't seem to be typed.

Maybe I can also take a look next week (this week is unfortunately quite busy).

But even if we don't figure that out ... your library is super nice. I use it for all my threading / multiprocessing stuff. Thanks for that!

@noxdafox
Copy link
Owner

noxdafox commented Dec 2, 2023

Also, the function parameters of @concurrent.process, e.g., daemon don't seem to be typed.

We cannot infer during load time the arguments as the decorator might be called as such:

@concurrent.process
def function():
    return

@concurrent.process(timeout=10)
def function():
    return

def myfunction():
    return

function = concurrent.function(myfunction)
function = concurrent.function(myfunction, timeout=10)

In all these cases, the arguments and keyword arguments might be different depending on the situation. For example, if you use the @decorator syntax, the function is not passed to the decorator immediately but only during its invocation. Vice-versa, if your use the decorator as a function, the actual callable is passed right away.

Hence, the @concurrent.* and @asynchronous.* decorators must parse the arguments and keyword arguments dynamically. Therefore, we cannot annotate the decorator keyword arguments.

In other words, we cannot do:

def process(function, timeout: int = None, daemon: bool = True, ...):
     ...

But we have to do this:
https://github.com/noxdafox/pebble/blob/master/pebble/concurrent/process.py#L55

I was reading a bit around regarding this problem and found few related issues on the pyright itself.
microsoft/pyright#774
microsoft/pyright#2142

It might be quite hard to get linters to infer the types correctly.

noxdafox added a commit that referenced this issue Dec 6, 2023
@noxdafox
Copy link
Owner

noxdafox commented Dec 7, 2023

Did some small fixes but it didn't work.

There is some good hope when looking at :https://peps.python.org/pep-0612/

But I could not get it work straight away. Anyways, as it's supported from 3.10 onwards it makes little sense to add it right now as we need to guarantee a certain degree of backwards compatibility.

I will try this again in some time when the platforms will be more mature.

@noxdafox noxdafox closed this as completed Dec 7, 2023
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

2 participants