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

Argument missing for parameter "f" Pylance [reportCallIssue] #511

Open
kler opened this issue Dec 20, 2024 · 5 comments
Open

Argument missing for parameter "f" Pylance [reportCallIssue] #511

kler opened this issue Dec 20, 2024 · 5 comments
Labels

Comments

@kler
Copy link

kler commented Dec 20, 2024

  • Python State Machine version: 2.5.0
  • Python version: Python 3.13.1 (also reproduced in Python 3.10.12)
  • Operating System: Darwin arm64 24.1.0
  • Visual Studio Code
    Version: 1.96.0 (Universal)
    Commit: 138f619c86f1199955d53b4166bef66ef252935c
    Date: 2024-12-11T02:29:09.626Z
    Electron: 32.2.6
    ElectronBuildId: 10629634
    Chromium: 128.0.6613.186
    Node.js: 20.18.1
    V8: 12.8.374.38-electron.0
    OS: Darwin arm64 24.1.0
  • Pylance language server 2024.12.1 (pyright version 1.1.389, commit ce1325fc)

Description

I apologize in advance if this is outside scope of this project.

Since python-statemachine v2.5.0 VS Code displays a warning on lines executing transitions:

sm = PylanceWarningStateMachine()
sm.run() # <-- this line

The warning is:

image

Argument missing for parameter "f" Pylance reportCallIssue

I haven't figured out how Pylance gets to this conclusion. The f must come from statemachine/transition_mixin.py, which is the only file referencing a parameter named f.

What I also have noticed is that this warning doesn't appear in a VS Code vanilla configuration, but only after certain settings are made.

For instance adding this to the project's pyproject.toml will make VS Code display the warning:

[tool.pyright]
exclude = [
    "**/data",
    "**/__pycache__"
]

In fact, just add this line will make VS Code display the warning:

[tool.pyright]

Again, perhaps this is a bug in Pylance / Pyright for what I know, but the warning did not appear in v2.4.0. So in case you have an idea how to mitigate it, by changing the code or IDE setup, please share your thoughts :)

@kler
Copy link
Author

kler commented Dec 20, 2024

The static code analyzer is tracing the sm.run() below to AddCallbacksMixin.__call__ function.

Example code:

from statemachine import State, StateMachine
class PylanceWarningStateMachine(StateMachine):
    init = State(initial=True)
    running = State()
    completed = State(final=True)

    run = (
        init.to(running)
    )
    complete = (
        running.to(completed)
    )

if __name__ == "__main__":
    sm = PylanceWarningStateMachine()
    sm.run()
    sm.complete()

statemachine/transition_mixin.py

class AddCallbacksMixin:
    ...
    def __call__(self, f):
        return self._add_callback(f, CallbackGroup.ON, is_event=True)
...

I verified by renaming the f in the __call__, and this change is reflected in the Pylance warning:

Modified code:

class AddCallbacksMixin:
    ...
    def __call__(self, itsThisF):
        return self._add_callback(itsThisF, CallbackGroup.ON, is_event=True)
...

image

@fgmacedo
Copy link
Owner

fgmacedo commented Dec 20, 2024

Hi @kler, thanks for taking the time to report this. I was able to reproduce the issue by just adding [tool.pyright] to the project's pyproject.toml.

Context of the problem

When a transition is declared, it can be assigned to a class-level attribute, which later will be used as an Event name.

In your example (see the comments):

from statemachine import State
from statemachine import StateMachine


class PylanceWarningStateMachine(StateMachine):
    init = State(initial=True)
    running = State()
    completed = State(final=True)

    run = init.to(running)

    complete = running.to(completed)
    # At this point, when the body of the `StateMachine` is being evaluated,
    # the  `run` and `complete` attributes are still instances of `TransitionList` returned by
    # the `init.to(running)` and `running.to(completed)` calls.
    print("Type of `run` at class body is ", type(run))


# At this point, the `PylanceWarningStateMachine` body was fully evaluated, and transformed
# by the `StateMachineMetaclass`, so now `run`  and `complete` are instances of `Event`.
print("Type of `run` after the final class evaluation is ", type(PylanceWarningStateMachine.run))


if __name__ == "__main__":
    sm = PylanceWarningStateMachine()
    sm.run()
    sm.complete()

Outputs:

❯ uv run tests/examples/types_machine.py
Type of `run` at class body is  <class 'statemachine.transition_list.TransitionList'>
Type of `run` after the final class evaluation is  <class 'statemachine.event.Event'>

We say run is an event name, and init.to(running) creates a transition that is returned as a TransitionList instance.

So when the body of a StateMachine class is evaluated, run is an instance of TransitionList. But the body attributes are evaluated and transformed by a metaclass, and then run becomes an Event instance.

The IDE and the Pyright are unable to follow this processing and still think that run is a TransitionList.

The __call__ of a transition list is to allow the declaration of callbacks using the decorator syntax, so it expects a function/method, so this is why it expects f as a parameter.

    run = (
        init.to(running)
    )

    @run  # <- "calling" a `TransitionList` allows registering action callbacks
    def do_run(self):
        print("running!")

Fix

Since these tools don't "run" the code, AFAIK there's no way to teach them that the result is an Event instead of a TransitionList. But to help the linter stop complaining, we can change the signature of the __call__ to the generic *args, **kwargs format, something like this:

from .i18n import _

    def __call__(self, *args, **kwargs) -> Any:
        if len(args) == 1 and callable(args[0]):
            return self._add_callback(args[0], CallbackGroup.ON, is_event=True)
        raise ValueError(
            _("Unsupported call signature. Calling %s only to register callbacks").format(
                self.__class__.__name__
            )
        )

You can apply this monkey patch until a new release arrives.

Please let me know if it worked.

@kler
Copy link
Author

kler commented Dec 21, 2024

I've verified and it works beautifully!

@fgmacedo
Copy link
Owner

Nice! Another and maybe better approach could be to use the explicit Event creation, so the linter will be happy:

from statemachine import State, StateMachine, Event
class PylanceWarningStateMachine(StateMachine):
    init = State(initial=True)
    running = State()
    completed = State(final=True)

    run = Event(
        init.to(running)
    )
    complete = Event(
        running.to(completed)
    )

if __name__ == "__main__":
    sm = PylanceWarningStateMachine()
    sm.run()
    sm.complete()

@kler
Copy link
Author

kler commented Dec 21, 2024

That's a good suggestion as well, though I do like the raw beauty of

run = ( ... ) 🤓

If we could get your first fix into the repo that would be a great 🎁

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

No branches or pull requests

2 participants