Provide features to ease static type checking about transitions HOT 10 OPEN

Hi @sy-be,

Could it be that stub files are outdated? Thank you!

they are not outdated but could use improvements. As transitions was initially built rather "duck-ish" and very agnostic towards passed arguments, "post-mortem typing" isn't trivial. Furthermore, it reveals some weaknesses the inheritance approach which heavily relies on overriding has (e.g. sync to async or Liskov substitution principle violations) that are not easy to fix. That's also a good thing but requires some refactoring.

I made some minor changes in master (EventData.machine is not optional any longer, Transitions.dest might be None) and when you are okay with ignoring override issues, this should appease mypy.

from transitions.extensions.asyncio import AsyncTransition, AsyncEventData


class MyTransition(AsyncTransition):
    async def _change_state(self, event_data: AsyncEventData) -> None:   # type: ignore[override]
        assert self.dest is not None
        if hasattr(event_data.machine, "model_graphs"):  # [1]
            graph = event_data.machine.model_graphs[id(event_data.model)]
            graph.reset_styling()
            graph.set_previous_transition(self.source, self.dest)
        await event_data.machine.get_state(self.source).exit(event_data)
        event_data.machine.set_state(self.dest, event_data.model)
        event_data.update(getattr(event_data.model, event_data.machine.model_attribute))
        await event_data.machine.get_state(self.dest).enter(event_data)

However, I guess that block [1] will be moved somewhere else because the graph should be updated even if no state change happened during an internal transition. _state_change should only be called if an actual state change is happening. My first guess would be to add a graph update function to machine.before_state_change or machine.after_state_change.

from transitions.

Hi @sy-be,

Any plans on releasing these changes?

end of this week. I need to check open issues and see whether there are some major issues to be tackled. features will be postponed to the next release though.

from transitions.

Thanks! It'd also help if the code would be typed in general

from transitions.

Hello @sy-bee,

Thanks! It'd also help if the code would be typed in general

currently type information are stored in stub (*.pyi) files for backward compatibility reasons. Frameworks like MyPy are able to consider stub files while type checking. Some IDEs (like PyCharm) usually also consider stub files without issues. Some other IDEs (e.g. VSCode) only consider stub files of imported functions (see picture below).

from transitions.

Thank you for quick responese! I see it now, in my case I am using VScode with mypy and as you mentioned - VSCode does not always consider stub files. Now the issue I'm having (apart from mentioned runtime decorated functions) is related to subclassing AsyncTransition in order to override its _change_state() method. In my override method I keep it mostly as is apart from one extra call. The method signature remains the same and no matter how I try to annotate its return type - mypy returns errors. For clarity, I've kept the method as is:

from transitions import EventData
from transitions.extensions.asyncio import AsyncTransition


class MyTransition(AsyncTransition):
    async def _change_state(self, event_data: EventData) -> None:
        if hasattr(event_data.machine, "model_graphs"):
            graph = event_data.machine.model_graphs[id(event_data.model)]
            graph.reset_styling()
            graph.set_previous_transition(self.source, self.dest)
        await event_data.machine.get_state(self.source).exit(event_data)
        event_data.machine.set_state(self.dest, event_data.model)
        event_data.update(getattr(event_data.model, event_data.machine.model_attribute))
        await event_data.machine.get_state(self.dest).enter(event_data)

with the return type set to None mypy returns:

my_transition.py:6: error: Return type "Coroutine[Any, Any, None]" of "_change_state" incompatible with return type "None" in supertype "Transition"  [override]
my_transition.py:8: error: Item "None" of "Machine | None" has no attribute "model_graphs"  [union-attr]
my_transition.py:11: error: Incompatible types in "await" (actual type "Any | None", expected type "Awaitable[Any]")  [misc]
my_transition.py:11: error: Item "None" of "Machine | None" has no attribute "get_state"  [union-attr]
my_transition.py:12: error: Item "None" of "Machine | None" has no attribute "set_state"  [union-attr]
my_transition.py:13: error: Item "None" of "Machine | None" has no attribute "model_attribute"  [union-attr]
my_transition.py:14: error: Incompatible types in "await" (actual type "Any | None", expected type "Awaitable[Any]")  [misc]
my_transition.py:14: error: Item "None" of "Machine | None" has no attribute "get_state"  [union-attr]

I've tried different return type, e.g. with Coroutine[Any, Any, None] I get

...
my_transition.py:7: error: Return type "Coroutine[Any, Any, Coroutine[Any, Any, None]]" of "_change_state" incompatible with return type "Coroutine[Any, Any, None]" in supertype "AsyncTransition"  [override]
my_transition.py:7: error: Return type "Coroutine[Any, Any, Coroutine[Any, Any, None]]" of "_change_state" incompatible with return type "None" in supertype "Transition"  [override]
...

This is apart from the rest of the typing issues with missing attributes.

Could it be that stub files are outdated? Thank you!

from transitions.

Excellent! Thanks for your input! I like the library a lot, with better typing it'd be even better. Any plans on releasing these changes? I'm working on adding types to a large project and ideally would love to minimise the amount of type: ignores. There aren't many related to transitions, mostly this subclass I mentioned above and the attr-defined ones - which I can workaround with a decorator theoretically.

from transitions.

Hello @sy-be, @quantitative-technologies, @lucaswhipple-sfn! I am working on some utilities that hopefully ease the work with transitions when doing static type checks. The current state of the process can be found in the dev-experimental-model-creation branch.

Here is the relevant section from the documentation:

Typing support

As you probably noticed, transitions uses some of Python's dynamic features to give you handy ways to handle models. However, static type checkers don't like model attributes and methods not being known before runtime. Historically, transitions also didn't assign convenience methods already defined on models to prevent accidental overrides.

But don't worry! You can use the machine constructor parameter model_override to change how models are decorated. If you set model_override=True, transitions will only override already defined methods. This prevents new methods from showing up at runtime and also allows you to define which helper methods you want to use.

from transitions import Machine

# Dynamic assignment
class Model:
    pass

model = Model()
default_machine = Machine(model, states=["A", "B"], transitions=[["go", "A", "B"]], initial="A")
print(model.__dict__.keys())  # all convenience functions have been assigned
# >> dict_keys(['trigger', 'to_A', 'may_to_A', 'to_B', 'may_to_B', 'go', 'may_go', 'is_A', 'is_B', 'state'])
assert model.is_A()  # Unresolved attribute reference 'is_A' for class 'Model'


# Predefined assigment: We are just interested in calling our 'go' event and will trigger the other events by name
class PredefinedModel:
    # state (or another parameter if you set 'model_attribute') will be assigned anyway 
    # because we need to keep track of the model's state
    state: str

    def go(self) -> bool:
        raise RuntimeError("Should be overridden!")

    def trigger(self, trigger_name: str) -> bool:
        raise RuntimeError("Should be overridden!")


model = PredefinedModel()
override_machine = Machine(model, states=["A", "B"], transitions=[["go", "A", "B"]], initial="A", model_override=True)
print(model.__dict__.keys())
# >> dict_keys(['trigger', 'go', 'state'])
model.trigger("to_B")
assert model.state == "B"

If you want to use all the convenience functions and throw some callbacks into the mix, defining a model can get pretty complicated when you have a lot of states and transitions defined.
The method generate_base_model in transitions can generate a base model from a machine configuration to help you out with that.

from transitions.experimental.utils import generate_base_model
simple_config = {
    "states": ["A", "B"],
    "transitions": [
        ["go", "A", "B"],
    ],
    "initial": "A",
    "before_state_change": "call_this",
    "model_override": True,
} 

class_definition = generate_base_model(simple_config)
with open("base_model.py", "w") as f:
    f.write(class_definition)

# ... in another file
from transitions import Machine
from base_model import BaseModel

class Model(BaseModel):  #  call_this will be an abstract method in BaseModel

    def call_this(self) -> None:
        # do something  

model = Model()
machine = Machine(model, **simple_config)

Where I need your feedback

I plan to add some model decorators that enables the definition of trigger/events in a model class. I have two approaches: a) via function decorators and b) via functions that return a callable placeholder. This is how they could look like:

from transitions.experimental.utils import trigger_decorator, trigger, transition, with_trigger_decorator
from transitions import Machine


class Model:

    @trigger_decorator(source="A", dest="B")  # version A
    @trigger_decorator(source="B", dest="C")
    def foo(self):
        raise RuntimeError("Trigger was not initialized correctly.")

    bar = trigger(transition(source="B", dest="A", conditions=lambda: False),
                  transition(source="B", dest="A"))  # version B


@with_trigger_decorator
class MyMachine(Machine):
    pass


model = Model()
machine = MyMachine(model, states=["A", "B"], initial="A")
model.foo()
model.bar()
assert model.state == "A"

What do you think?

Does this look useful to you?

Which version to you prefer?

What do you think about model_override and generate_base_model?

Also let me know if you have some further ideas that would ease the work with transitions when it comes to typing and beyond 👋

from transitions.

Excellent! Thank you for this consideration.

I quite like the idea of generate_base_model - to make a static reference object that can be easily modified either at runtime or in a CI step is very useful - I currently update my diagram as a CI Step in my pipeline, and could imagine doing the same for generate_base_model to update my static, reference model configuration.

For better or worse, I generally don't make a separate Model class when defining my state machine - I typically just override the __init__ method and do things there, which may be anathemic to your design but is what I ended up doing. For this reason, model_override does not seem useful for MY use case, though others I'm sure would appreciate it.

My $.02 on Version A vs. Version B is that version A seems to have a method definition syntax that is less hidden to me that the magic of declaring a trigger as a class variable (which is what I think version B is doing?) but my limited knowledge of the behind-the-scenes magic may be compounding my opinion.

Thank you for doing this work and I hope my feedback helps.

from transitions.

Hello @lucaswhipple-sfn and thank you for the feedback.

I generally don't make a separate Model class when defining my state machine - I typically just override the init method and do things there

do you mean you inherit from (Hierarchical/Async)Machine?

For this reason, model_override does not seem useful for MY use case, though others I'm sure would appreciate it.

It depends. Machine.model_override is checked inside Machine._checked_assignment so whenever a model (or the machine itself) is decorated with convenience functions (e.g. when adding a transition with a new trigger or a new state or a new model). So even if you inherit from a Machine: If you don't pass a dedicated model to super it will treat machine as a model and chose the decoration strategy based on model_override:

from transitions import Machine


# You could also use a BaseModel here
# class MyMachine(BaseModel, Machine):
class MyMachine(Machine):
    def __init__(self):
        super(MyMachine, self).__init__(states=["A", "B"], initial="A", model_override=True)
        self.add_transition("go", "A", "B")

    def go(self) -> bool:
        raise RuntimeError("Should be overridden!")

    def is_A(self) -> bool:
        raise RuntimeError("Should be overridden!")


m = MyMachine()
assert m.go()
assert not m.is_A()
# m.is_B()  # AttributeError: 'is_B' does not exist on

My $.02 on Version A vs. Version B is that version A seems to have a method definition syntax that is less hidden to me that the magic of declaring a trigger as a class variable (which is what I think version B is doing?)

You are right. Version B more or less returns a Callable placeholder to a class variable. IDEs and language servers should treat version A and B almost as the same. VSCode highlights version B as a variable though.

I also prefer version A when it comes to comprehensibility. When many transitions need to be defined, version B will be more compact as you don't need empty lines between definitions and could also add multiple transitions in one line.

from transitions.

Thank you for your work! I like the model_override parameter, I prefer to define my methods and their types and then call them directly or via trigger().

On generate_base_model - it's useful in certain scenarios, but I generally don't use autogenerated code in my code, it introduces ambiguity and extra steps to ensure the generated code is up to date.

As per version A or B for triggers, I'd generally prefer version A as it's more clearly defined and improves visibility. However in the project I'm working on, historically the whole model was designed in a very unusual way, so I doubt I'll be able to use any of these approaches unless I refactor the code (which I hope to do one day!).

On another note

I tried running from the branch and also on version 0.9.1, I now get extra mypy warnings like:

• Argument 1 to "get" of "dict" has incompatible type "str | None"; expected "str" [arg-type] - this is because Transition.dest can sometimes be None. How can we transition if there's no destination?
• On the following function:

class MyTransition(AsyncTransition):
    async def _change_state(self, event_data: AsyncEventData) -> None:   # type: ignore[override]
        assert self.dest is not None
        if hasattr(event_data.machine, "model_graphs"):
            graph = event_data.machine.model_graphs[id(event_data.model)]
            graph.reset_styling()
            graph.set_previous_transition(self.source, self.dest)
        await event_data.machine.get_state(self.source).exit(event_data)
        event_data.machine.set_state(self.dest, event_data.model)
        event_data.update(getattr(event_data.model, event_data.machine.model_attribute))
        await event_data.machine.get_state(self.dest).enter(event_data)

I get errors (line numbers aligned):

8: error: "exit" of "State" does not return a value (it only ever returns None)  [func-returns-value]
9: error: Argument 1 to "set_state" of "Machine" has incompatible type "str | None"; expected "str | Enum | State"  [arg-type]
11: error: "enter" of "State" does not return a value (it only ever returns None)  [func-returns-value]
11: error: Argument 1 to "get_state" of "Machine" has incompatible type "str | None"; expected "str | Enum"  [arg-type]

lines 9 and the last 11 are related to the above - transition.dest potentially being None
other lines are due to Machine.get_state and Machine.set_state do not return any awaitable (they are normal functions)

This second issue was there before, I guess the code needs updating as well as pyi files.

from transitions.