pytransitions / transitions

A lightweight, object-oriented finite state machine implementation in Python with many extensions
MIT License
5.48k stars 525 forks source link

Pylance attributeerror on generated members #654

Closed lucaswhipple-sfn closed 2 months ago

lucaswhipple-sfn commented 2 months ago

Describe the bug I am using VSCode and pylance to develop a state machine. I am running into an issue where my IDE is not identifying what I suspect are generated members of a class during static type checking, and furthermore not allowing for them to be selected in code completion.

I admit this may be more of a pylance issue, but I wanted to raise it here in case it was not known or there was an easy fix.

The specific pylance error I am getting is "Cannot access member "state" for type "MinimalStateMachine"   Member "state" is unknown" - this is documented in the pylance docs here - https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportAttributeAccessIssue

Minimal working example from transitions.extensions.diagrams import GraphMachine import enum

class States(enum.Enum): IDLE = 0 HOME = 1 FAULT = 2 COMPLETE = 3

class MinimalStateMachine: def init(self): self.machine = GraphMachine(model=self, states=States, initial=States.IDLE, send_event= True) self.machine.add_transition("fault", source=States._membernames, dest=States.FAULT) # type: ignore

if name == "main": model = MinimalStateMachine() print(model.state) print(model.fault()) print(model.state)

Expected behavior I expected the model.state, model.fault() and model.state lines to be detected by pylance and available for code completion. However, as generated members (?) they appear as errors, though the code works.

Additional context an image of the hoverover is presented here. image

aleneum commented 2 months ago

Hello @lucaswhipple-sfn,

yeah, that's a major drawback of the dynamically decorated attributes. I am open for suggestions about how to deal with this. Maybe this post gives you some ideas:

What is the recommended way to deal with lack of type ahead/code completion in the IDE for "triggers"?

You can add docstrings to your models that IDEs can use for code completion (see https://github.com/pytransitions/transitions/issues/426) or you can customize a Machine to generate docstrings for you (see https://github.com/pytransitions/transitions/issues/383). Considering naming, you can adjust method naming by overidding Machine._add_model_to_state (see https://github.com/pytransitions/transitions/issues/385).

lucaswhipple-sfn commented 2 months ago

Regrettably from my searching I don't see a simple way to do this, as it appears that pylance won't lookahead for generated member, presumably because it requires too much library introspection? My IDE not knowing that the model has the trigger attributes before runtime is tricky for debugging, and I don't know enough about how it preforms lookahead to offer good suggestions. I don't think the suggestions you proposed in your response will help with this either.

Your DocMachine is pretty interesting but it looks like that still won't convince pylance to walk through dynamically decorated attributes.

python-statemachine approaches this differently and has all of the transitions defined as class attributes, but I think that would be a fairly signifigant overhaul of Transition's code, and as such is unreasonable.

aleneum commented 2 months ago

transitions contains an internal method to prevent accidental overrides of model attributes. You can rather easily workaround it. If you are willing to define your Model methods manually in advance to have better Pylance suggestions, you could do it this way:

from transitions import Machine

class OverridingMachine(Machine):

    def _checked_assignment(self, model, name, func):
        setattr(model, name, func)

class Model:

    def event_a(self) -> bool:
        pass

    def event_b(self) -> bool:
        pass

    def is_A(self) -> bool:
        pass

    def is_B(self) -> bool:
        pass

    def is_C(self) -> bool:
        pass

    def state(self) -> str:
        pass

states = ["A", "B", "C"]
transitions = [
    ["event_a", "A", "B"],
    ["event_b", "B", "C"],
    ["event_a", "C", "A"],
]

model = Model()
machine = OverridingMachine(model, states=states, transitions=transitions,
                            initial="A")
model.event_a()
assert model.is_B()
model.event_b()
assert model.is_C()
model.event_a()
assert model.is_A()
assert model.state == "A"

This is how it looks for me with PyCharm:

Screenshot 2024-04-19 at 14 02 48 Screenshot 2024-04-19 at 14 03 00 Screenshot 2024-04-19 at 14 04 06

Instead of pass you could also add a comment which might even add more contextual information. If you just want to silence warnings about dynamically generated methods, have a look at this particular comment again:

https://github.com/pytransitions/transitions/issues/426#issuecomment-629814250

lucaswhipple-sfn commented 2 months ago

I think this is a reasonable solution, but it takes a lot of the magic of Transitions away and requires a lot of boilerplate code. I almost wish there were a decorator to identify that a model method was a transition or event.

I'm going to keep testing with it, and if I have success with some kind of decorator I will respond to this thread. Thank you for the detailed feedback and for the potential solution!

aleneum commented 2 months ago

Yeah, a decorator would be nice.

What would you think about something like this?

from transitions import Machine
from typing import Optional, Callable, Union, List, TYPE_CHECKING
from functools import  wraps

if TYPE_CHECKING:
    from transitions.core import StateIdentifier, CallbacksArg

# decorator draft
def transition(source: Union["StateIdentifier", List["StateIdentifier"]] = None,
               dest: Optional[str] = None,
               conditions: Optional["CallbacksArg"] = None,
               unless: Optional["CallbacksArg"] = None) -> Callable[[Callable[..., bool]], Callable[..., bool]]:

    def _outer(trigger: Callable[..., bool]) -> Callable[..., bool]:
        wraps(trigger)

        def _inner(self, *args, **kwargs) -> bool:
            name = trigger.__name__
            self.add_transition(name, source, dest, conditions, unless)
            return self.trigger(name, *args, **kwargs)
        return _inner
    return _outer

class MyMachine(Machine):

    state: str = ""

    def _checked_assignment(self, model, name, func):
        setattr(model, name, func)

    @transition(source="A", dest="B")
    def event_a(self) -> bool:
        """Triggered when 'a' happens."""

    @transition(source="*", dest="A", unless=lambda invalid=False: invalid is True)
    def reset(self):
        """Reset to initial state 'A'"""

    def on_enter_A(self, invalid=False):
        print(f"Entered A!")

machine = MyMachine(states=["A", "B"], initial="A", auto_transitions=False)
machine.event_a()
assert machine.state == "B"
machine.reset(invalid=True) 
assert machine.state == "B"
machine.reset(invalid=False)  # >>> Entered A
assert machine.state == "A"

Or would you prefer the style python-statemachine uses like:

class Model:
    # ...
    event_a = transition(source="A", dest="B", unless=...)

I guess both styles could be used to add more reliable (static) type information.

lucaswhipple-sfn commented 2 months ago

The decorator is a very interesting concept - I haven't thought through the potential ramifications of the decorator, but it solves my type-checker problem! Running your sample code through my IDE gives me the expected result of knowing it is a member as well.

I personally DON'T prefer the explicit declaration of transitions as done in python-statemachine, because then you have to declare a transition and then also potentially do a bunch of checking in the on-enter generated methods... but it would also solve the initial problem that I raised.

I may end up using this decorator in my code - I'd be curious to see it in future releases!