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

Previous transition before/on/after methods are called again, sometimes #308

Closed
Kevin-Prichard opened this issue Jan 10, 2023 · 9 comments · Fixed by #310
Closed

Previous transition before/on/after methods are called again, sometimes #308

Kevin-Prichard opened this issue Jan 10, 2023 · 9 comments · Fixed by #310
Assignees

Comments

@Kevin-Prichard
Copy link

Kevin-Prichard commented Jan 10, 2023

  • Python State Machine version: '0.9.0'
  • Python version: '3.8.10 (default, Nov 14 2022, 12:59:47) \n[GCC 9.4.0]'
  • Operating System: 'Linux 5.15.0-57-generic 20.04.1-Ubuntu SMP Wed Nov 30 13:40:16 UTC 2022 x86_64'

Description

While exploring this really promising module, I coded up a state machine to put it through some paces. It has states (state1, state2, state3, state4), transitions (trans12, trans23, trans34), and cycle defined (trans12 | trans23 | trans34).

I noticed what might be either an incorrect assumption on my part, or possibly a bug: the first transition methods (before_/on_/after_ methods) were getting called after that transition has already passed, when later transitions' methods were also being called.

Demo code-

from statemachine import StateMachine, State


class TestSM(StateMachine):
    state1 = State('state1', initial=True)
    state2 = State('state2')
    state3 = State('state3')
    state4 = State('state4', final=True)

    trans12 = state1.to(state2)
    trans23 = state2.to(state3)
    trans34 = state3.to(state4)

    cycle = trans12 | trans23 | trans34

    def before_cycle(self):
        print("before cycle")

    def on_cycle(self):
        print("on cycle")

    def after_cycle(self):
        print("after cycle")

    def on_enter_state1(self):
        print('enter state1')

    def on_exit_state1(self):
        print('exit state1')

    def on_enter_state2(self):
        print('enter state2')

    def on_exit_state2(self):
        print('exit state2')

    def on_enter_state3(self):
        print('enter state3')

    def on_exit_state3(self):
        print('exit state3')

    def on_enter_state4(self):
        print('enter state4')

    def on_exit_state4(self):
        print('exit state4')

    def before_trans12(self):
        print('before trans12')

    def on_trans12(self):
        print('on trans12')

    def after_trans12(self):
        print('after trans12')

    def before_trans23(self):
        print('before trans23')

    def on_trans23(self):
        print('on trans23')

    def after_trans23(self):
        print('after trans23')

    def before_trans34(self):
        print('before trans34')

    def on_trans34(self):
        print('on trans34')

    def after_trans34(self):
        print('after trans34')

Paste the command(s) you ran and the output.

>>> m = ubr.TestSM()
enter state1

>>> m.is_state1, m.is_state2, m.is_state3, m.is_state4, m.current_state ; _ = m.cycle()
(True, False, False, False, State('state1', id='state1', value='state1', initial=True, final=False))
before cycle
before trans12
exit state1
on cycle
on trans12
enter state2
after cycle
after trans12

>>> m.is_state1, m.is_state2, m.is_state3, m.is_state4, m.current_state ; _ = m.cycle()
(False, True, False, False, State('state2', id='state2', value='state2', initial=False, final=False))
before cycle
before trans12
before trans23
exit state2
on cycle
on trans12
on trans23
enter state3
after cycle
after trans12
after trans23

>>> m.is_state1, m.is_state2, m.is_state3, m.is_state4, m.current_state ; _ = m.cycle()
(False, False, True, False, State('state3', id='state3', value='state3', initial=False, final=False))
before cycle
before trans12
before trans34
exit state3
on cycle
on trans12
on trans34
enter state4
after cycle
after trans12
after trans34

>>> m.is_state1, m.is_state2, m.is_state3, m.is_state4, m.current_state
(False, False, False, True, State('state4', id='state4', value='state4', initial=False, final=True))

As mentioned, before_trans12, on_trans12 and after_trans12 continue firing after their turn is finished. Did I misconfigure something in my StateMachine subclass?

I spent some time debug-stepping through metaclass code while importing TestSM. Nothing conclusive, except that the Transition instances, after the first one, always start out containing "cycle trans12" before the current transition key is added to self._events.

For example, repr of trans23 after initialization, see how event='cycle trans12 trans23' ? That don't seem right.

Transition(State('state2', id='state2', value='state2', initial=False, final=False), State('state3', id='state3', value='state3', initial=False, final=False), event='cycle trans12 trans23')

I'll wait for a reply before continuing diagnosis.

-Kevin

Repository owner deleted a comment from brunolnetto Jan 10, 2023
@fgmacedo
Copy link
Owner

fgmacedo commented Jan 11, 2023

Hi @Kevin-Prichard , thanks for getting in touch.

This is the resulting SM from your example:

issue308

So what's happening here?

The example binds the same set of transitions to multiple events, and also merges the list of transitions.

These lines:

    trans12 = state1.to(state2)  # this returns a `TransitionList` that is bound to the name `trans12`
    trans23 = state2.to(state3)  # this returns a `TransitionList` that is bound to the name `trans23`
    trans34 = state3.to(state4)  # this returns a `TransitionList` that is bound to the name `trans23`

   # `TransitionList` implements the `|` or operator, and the boolean expression is evaluated from left to right,
   # Here I've found a bug, thanks! The `|` instead of returning a new object, was merging the right operand into itself.

    cycle = trans12 | trans23 | trans34 
  1. state1.to(state2) bind to trans12 and cycle
  2. state2.to(state3) bind to trans23 and cycle
  3. state3.to(state4) bind to trans34 and cycle

So instead of two transitions triggered by distinct events, you got the same transition triggered by multiple events.

Bug found! The TransitionList.__or__ was merging the result on itself instead of returning a new object.

And this is what happened when I fixed the TransitionList.__or__ operator:

issue308_fix_transition_list

Still not your expected behavior. But. Now we have a comprehensive explanation: Reusing the same TransitionList binds the transitions to multiple events.

It's the equivalent of:

class TestSM(StateMachine):
    state1 = State('state1', initial=True)
    state2 = State('state2')
    state3 = State('state3')
    state4 = State('state4', final=True)

    state1.to(state2, event="trans12 cycle")
    state2.to(state3, event="trans23 cycle")
    state3.to(state4, event="trans34 cycle")

And indeed is your expected behaviur if I got it right.

Alternative

~We cannot reuse the same transition definitions across events to get the desired behavior. ~

So instead of reusing the previous transitions, you can define new ones, and bind the new transitions to the cycle event:

from statemachine import StateMachine, State


class TestSM(StateMachine):
    state1 = State('state1', initial=True)
    state2 = State('state2')
    state3 = State('state3')
    state4 = State('state4', final=True)

    trans12 = state1.to(state2)
    trans23 = state2.to(state3)
    trans34 = state3.to(state4)

    cycle = state1.to(state2) | state2.to(state3) | state3.to(state4)

This is the SM fixed with the new transitions declaration:

issue308_fixed

Thanks for giving StateMachine a try. Let me know if you have any other issues. I'm planning to release the develop branch as 1.0 soon.

This is the Actions docs improved: https://python-statemachine.readthedocs.io/en/develop/actions.html

Feel free to reopen or reply to this or another issue.

Best regards,
Fernando

EDIT: Updated now that I think that I understood your need.

@fgmacedo
Copy link
Owner

A file that reproduces the fix:

issue308.md

Issue 308

A StateMachine that exercices the example given on issue
#308.

>>> from statemachine import StateMachine, State

>>> class TestSM(StateMachine):
...     state1 = State('s1', initial=True)
...     state2 = State('s2')
...     state3 = State('s3')
...     state4 = State('s4', final=True)
...
...     trans12 = state1.to(state2)
...     trans23 = state2.to(state3)
...     trans34 = state3.to(state4)
...
...     cycle = state1.to(state2) | state2.to(state3) | state3.to(state4)
...     # cycle = trans12 | trans23 | trans34
...
...     def before_cycle(self):
...         print("before cycle")
...
...     def on_cycle(self):
...         print("on cycle")
...
...     def after_cycle(self):
...         print("after cycle")
...
...     def on_enter_state1(self):
...         print('enter state1')
...
...     def on_exit_state1(self):
...         print('exit state1')
...
...     def on_enter_state2(self):
...         print('enter state2')
...
...     def on_exit_state2(self):
...         print('exit state2')
...
...     def on_enter_state3(self):
...         print('enter state3')
...
...     def on_exit_state3(self):
...         print('exit state3')
...
...     def on_enter_state4(self):
...         print('enter state4')
...
...     def on_exit_state4(self):
...         print('exit state4')
...
...     def before_trans12(self):
...         print('before trans12')
...
...     def on_trans12(self):
...         print('on trans12')
...
...     def after_trans12(self):
...         print('after trans12')
...
...     def before_trans23(self):
...         print('before trans23')
...
...     def on_trans23(self):
...         print('on trans23')
...
...     def after_trans23(self):
...         print('after trans23')
...
...     def before_trans34(self):
...         print('before trans34')
...
...     def on_trans34(self):
...         print('on trans34')
...
...     def after_trans34(self):
...         print('after trans34')
...


Example given:

```py

>>> m = TestSM()
enter state1

>>> m._graph().write_png("issue308_fix_transition_list.png")

>>> m.is_state1, m.is_state2, m.is_state3, m.is_state4, m.current_state ; _ = m.cycle()
(True, False, False, False, State('s1', id='state1', value='state1', initial=True, final=False))
before cycle
exit state1
on cycle
enter state2
after cycle

>>> m.is_state1, m.is_state2, m.is_state3, m.is_state4, m.current_state ; _ = m.cycle()
(False, True, False, False, State('s2', id='state2', value='state2', initial=False, final=False))
before cycle
exit state2
on cycle
enter state3
after cycle

>>> m.is_state1, m.is_state2, m.is_state3, m.is_state4, m.current_state ; _ = m.cycle()
(False, False, True, False, State('s3', id='state3', value='state3', initial=False, final=False))
before cycle
exit state3
on cycle
enter state4
after cycle

>>> m.is_state1, m.is_state2, m.is_state3, m.is_state4, m.current_state
(False, False, False, True, State('s4', id='state4', value='state4', initial=False, final=True))

@Kevin-Prichard
Copy link
Author

All understood, thanks for explaining, @fgmacedo . To clarify, the way I did it generated a .cycle() with a forward-accumulated call chain of transitions/actions, calling the current transition/action before/on/after handlers, as well as the previous transition handlers after they've already been called before, correct? (What is that useful for?)

I understand now that the way I defined cycle = trans12 | trans23 | ..., with pre-defined transitions, produced this behavior. Therefore, by giving cycle its own transition definitions, the back-calling won't happen. cycle = state1.to(state2) | ...

But, with cycle = state1.to(state2) | ..., I see that calls aren't made to my defined before/on/after transition/action methods on the subclass.

There's only enter_state<N>() and exit_state<N>() (and the generic methods like enter_cycle().) In behavior graphs, action takes place on the edge, not the vertex -of course, you know this. I want to invoke a custom method when a transition occurs from stateA to stateB. It seems odd that you would allow it for a cycle() defined via TransitionList.__or__(), but seemingly not allow it otherwise.

Unless I'm missing something? In your last example, we see the exit and entry of states. If I have to inspect kwargs on each enter_stateN() method and build a chain if/then, or a match/case, to figure out which edge was transited... that defeats the purpose of a state machine module, I would think.

To review, what I thought I saw, in the README and docs, was that custom methods per transition were allowed. After reading through everything, it looks like it's only allowed in two cases, 1) using the decorators on by state.to.itself() (e.g. @loop.before), or 2) when you've built a cycle = trans12 | trans23 | .. that uses pre-defined transition methods (in which case you get calls to previous transition custom methods, too, which doesn't work for me.)

Take the case of bank transactions. I want to implement behavior for each state transition. I don't want to re-execution a prior transition! That would mess with state. Maybe I have missed something.

@fgmacedo
Copy link
Owner

fgmacedo commented Jan 11, 2023

Hi @Kevin-Prichard , thanks for your time reviewing this.

Can you please elaborate an example like a test case with assertion to your expected behavior?

Trying to figure out generically with pseudo states, transitions and events is harder than with a concrete example. The way you explained if I understood right is how it worked after the fix on the OR operator (already merged).

which case you get calls to previous transition custom methods, too, which doesn't work for me

This was in fact a bug and the fix is already on the develop branch.

But trying to answer your question, it's possible to bind actions to events and for specific transitions.

Using a visual graph representation, a transition is the edge, the link between two states. An event is the thing that fired this transition, we use to write the name of the event on the edge.

The most common usage is like you said, bind actions by name convention on the event. This is due the fact that Transition by itself doesn't have a name, they receive the events attached that they can react to.

So, you can bind actions on the transition by using constructor params. Each transition can receive any number of callbacks:

  • Transition(source, target, on="on12", before="before12", after="after12", cond="can12")

These names will be searched on the StateMachine and on a model, if one is given.

The to and from_ helpers are actually wrappers to construct transitions and any keyword param received is passed to the transition constructor: s1.to(s2, on="on12", before="before12", after="after12", cond="can12")

Let me know if this helps.

Best,
Fernando

@fgmacedo fgmacedo reopened this Jan 11, 2023
@fgmacedo
Copy link
Owner

fgmacedo commented Jan 11, 2023

@Kevin-Prichard , now I think that I understood your expected behavior, and indeed the bug fixed at #310 resolves the issue.

Issue 308

A StateMachine that exercices the example given on issue #308. Now with the fix applied on #310.

>>> from statemachine import StateMachine, State

>>> class TestSM(StateMachine):
...     state1 = State('s1', initial=True)
...     state2 = State('s2')
...     state3 = State('s3')
...     state4 = State('s4', final=True)
...
...     trans12 = state1.to(state2)
...     trans23 = state2.to(state3)
...     trans34 = state3.to(state4)
...
...     cycle = trans12 | trans23 | trans34
...
...     def before_cycle(self):
...         print("before cycle")
...
...     def on_cycle(self):
...         print("on cycle")
...
...     def after_cycle(self):
...         print("after cycle")
...
...     def on_enter_state1(self):
...         print('enter state1')
...
...     def on_exit_state1(self):
...         print('exit state1')
...
...     def on_enter_state2(self):
...         print('enter state2')
...
...     def on_exit_state2(self):
...         print('exit state2')
...
...     def on_enter_state3(self):
...         print('enter state3')
...
...     def on_exit_state3(self):
...         print('exit state3')
...
...     def on_enter_state4(self):
...         print('enter state4')
...
...     def on_exit_state4(self):
...         print('exit state4')
...
...     def before_trans12(self):
...         print('before trans12')
...
...     def on_trans12(self):
...         print('on trans12')
...
...     def after_trans12(self):
...         print('after trans12')
...
...     def before_trans23(self):
...         print('before trans23')
...
...     def on_trans23(self):
...         print('on trans23')
...
...     def after_trans23(self):
...         print('after trans23')
...
...     def before_trans34(self):
...         print('before trans34')
...
...     def on_trans34(self):
...         print('on trans34')
...
...     def after_trans34(self):
...         print('after trans34')
...

Example given:

>>> m = TestSM()
enter state1

>>> m._graph().write_png("issue308_fix_transition_list.png")

>>> m.is_state1, m.is_state2, m.is_state3, m.is_state4, m.current_state ; _ = m.cycle()
(True, False, False, False, State('s1', id='state1', value='state1', initial=True, final=False))
before cycle
before trans12
exit state1
on cycle
on trans12
enter state2
after cycle
after trans12

>>> m.is_state1, m.is_state2, m.is_state3, m.is_state4, m.current_state ; _ = m.cycle()
(False, True, False, False, State('s2', id='state2', value='state2', initial=False, final=False))
before cycle
before trans23
exit state2
on cycle
on trans23
enter state3
after cycle
after trans23

>>> m.is_state1, m.is_state2, m.is_state3, m.is_state4, m.current_state ; _ = m.cycle()
(False, False, True, False, State('s3', id='state3', value='state3', initial=False, final=False))
before cycle
before trans34
exit state3
on cycle
on trans34
enter state4
after cycle
after trans34

>>> m.is_state1, m.is_state2, m.is_state3, m.is_state4, m.current_state
(False, False, False, True, State('s4', id='state4', value='state4', initial=False, final=True))

Resulting SM:
issue308_fix_transition_list

You can test this example locally placing the issue308.md file at tests/examples/issue308.md and running pytest at the root folder.

Best regards,
Fernando

@fgmacedo
Copy link
Owner

I've included a folder tests/testcases for narrative-style doc testing examples. The first example is the one found on this thread: https://github.com/fgmacedo/python-statemachine/blob/develop/tests/testcases/issue308.md

@Kevin-Prichard
Copy link
Author

That's it! It works as I was expecting now, in my local test cases. (I hope this change harmonizes with other developers using of this module in their projects.)

And the test added in #310 covers the use-case I was trying to explain. Beautiful.

Thank you.

@fgmacedo
Copy link
Owner

Nice! I'll close the issue for now. Feel free to reach out.

@fgmacedo
Copy link
Owner

fgmacedo commented Mar 5, 2023

@Kevin-Prichard In the upcoming release, I've added a condition on the callback to only run if the callback was associated with the expected event. So even sharing the same transition instance, the action will be only fired if the event name matches.

This is a kinda breaking change / fix for your use case: #365

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