Manually Defining Trigger Methods Breaks Declared State Transitions

[campellcl]

The documentation in the README states (the emphasis is mine):

Notice the shiny new methods attached to the Matter instance (evaporate(), ionize(), etc.). Each method triggers the corresponding transition. You don't have to explicitly define these methods anywhere; the name of each transition is bound to the model passed to the Machine initializer (in this case, lump).

I interpreted this to mean that you may define the trigger methods manually, but don't have to. If we don't define the trigger methods manually (as in the below example) the output matches what is expected:

Source code:

from transitions import State, Machine

class StateMachineModel:

    state = None

    def __init__(self):
        pass

    # def transition_one(self):
    #     print('transitioning states...')

    # def transition_two(self):
    #     print('transitioning states...')

if __name__ == '__main__':
    states = [State(name='A'), State(name='B'), State(name='C'), State(name='D')]
    transitions = [
        {'trigger': 'transition_one', 'source': 'A', 'dest': 'B'},
        {'trigger': 'transition_two', 'source': 'B', 'dest': 'C'},
        {'trigger': 'transition_three', 'source': 'C', 'dest': 'D'}
    ]
    state_machine_model = StateMachineModel()
    state_machine = Machine(model=state_machine_model, states=states, transitions=transitions, initial=states[0])
    print('state_machine_model (current state): %s' % state_machine_model.state)
    state_machine_model.transition_one()
    print('state_machine_model (current state): %s' % state_machine_model.state)
    state_machine_model.transition_two()
    print('state_machine_model (current state): %s' % state_machine_model.state)

Output:

state_machine_model (current state): A
state_machine_model (current state): B
state_machine_model (current state): C

However, if we do define the trigger methods manually (e.g. uncomment the trigger functions as shown below) the state machine fails to transition states:

from transitions import State, Machine

class StateMachineModel:

    state = None

    def __init__(self):
        pass

    def transition_one(self):
        print('transitioning states...')

    def transition_two(self):
        print('transitioning states...')

if __name__ == '__main__':
    states = [State(name='A'), State(name='B'), State(name='C'), State(name='D')]
    transitions = [
        {'trigger': 'transition_one', 'source': 'A', 'dest': 'B'},
        {'trigger': 'transition_two', 'source': 'B', 'dest': 'C'},
        {'trigger': 'transition_three', 'source': 'C', 'dest': 'D'}
    ]
    state_machine_model = StateMachineModel()
    state_machine = Machine(model=state_machine_model, states=states, transitions=transitions, initial=states[0])
    print('state_machine_model (current state): %s' % state_machine_model.state)
    state_machine_model.transition_one()
    print('state_machine_model (current state): %s' % state_machine_model.state)
    state_machine_model.transition_two()
    print('state_machine_model (current state): %s' % state_machine_model.state)

Unanticipated output:

state_machine_model (current state): A
transitioning states...
state_machine_model (current state): A
transitioning states...
state_machine_model (current state): A

If this is the expected behavior, please consider modifying the language in the README to make it clear that: in the event the user does explicitly define the trigger methods, they will have to manage the state transitions themselves.

If the behavior of explicitly defining trigger methods is permitted but I have not done it correctly, please add an example to the README which showcases the proper way to manually override the trigger methods while still preserving the functionality of automated state transitions.

[potens1]

What is strange is, this works :

import logging
from transitions import State, Machine
logging.basicConfig(level=logging.DEBUG)
logging.getLogger('transitions').setLevel(logging.DEBUG)

class StateMachineModel:

    state = None

    def __init__(self):
        pass

    def transition_one(self):
        print('transitioning states... B')

    def transition_two(self):
        print('transitioning states... C')

if __name__ == '__main__':
    states = [State(name='A'), State(name='B'), State(name='C'), State(name='D')]
    transitions = [
        {'trigger': 'transition_one', 'source': 'A', 'dest': 'B'},
        {'trigger': 'transition_two', 'source': 'B', 'dest': 'C'},
        {'trigger': 'transition_three', 'source': 'C', 'dest': 'D'}
    ]
    state_machine_model = StateMachineModel()
    state_machine = Machine(model=state_machine_model, states=states, transitions=transitions, initial=states[0])
    print('state_machine_model (current state): %s' % state_machine_model.state)
    state_machine_model.trigger("transition_one")
    print('state_machine_model (current state): %s' % state_machine_model.state)
    state_machine_model.trigger("transition_two")
    print('state_machine_model (current state): %s' % state_machine_model.state)

output:

WARNING:transitions.core:Model already contains an attribute 'transition_one'. Skip binding.
WARNING:transitions.core:Model already contains an attribute 'transition_two'. Skip binding.
state_machine_model (current state): A
DEBUG:transitions.core:Executed machine preparation callbacks before conditions.
DEBUG:transitions.core:Initiating transition from state A to state B...
DEBUG:transitions.core:Executed callbacks before conditions.
DEBUG:transitions.core:Executed callback before transition.
DEBUG:transitions.core:Exiting state A. Processing callbacks...
INFO:transitions.core:Exited state A
DEBUG:transitions.core:Entering state B. Processing callbacks...
INFO:transitions.core:Entered state B
DEBUG:transitions.core:Executed callback after transition.
DEBUG:transitions.core:Executed machine finalize callbacks
state_machine_model (current state): B
DEBUG:transitions.core:Executed machine preparation callbacks before conditions.
DEBUG:transitions.core:Initiating transition from state B to state C...
DEBUG:transitions.core:Executed callbacks before conditions.
DEBUG:transitions.core:Executed callback before transition.
DEBUG:transitions.core:Exiting state B. Processing callbacks...
INFO:transitions.core:Exited state B
DEBUG:transitions.core:Entering state C. Processing callbacks...
INFO:transitions.core:Entered state C
DEBUG:transitions.core:Executed callback after transition.
DEBUG:transitions.core:Executed machine finalize callbacks
state_machine_model (current state): C

Edit: it works, but not the way you expect, the "triggers" method are not executed at all (you can replace to code inside the method with 0/0, it will never raise exception)

[campellcl]

@potens1 That is odd... a logger shouldn't ever influence runtime behavior like that.

[potens1]

Sorry, I did not put emphasis on what I changed.

The logger is only there to see what's going on, but what I changed is doing trigger('trigger_name') instead of .trigger_name(). It does not invalidate your remark about doc wording, but I tried to show you the methods are not run at all with .trigger() but it is when you call the methods. The problem is your methods does not ask the state machine to change state at all (there is no implicit to_B when you call state_machine_model.transition_one(). So, if you change your code with this:

lass StateMachineModel:

    state = None

    def __init__(self):
        pass

    def transition_one(self):
        print('transitioning states... B')
        self.to_B()

    def transition_two(self):
        print('transitioning states... C')
        self.to_C()

it will work, but only because you use different triggers for different states. If you wanted to use the same trigger for multiple states, you have to extract the current state, transition, possible next states,... (you can have those easily if you set the machine to send_events=True, and then have everything in the attached event) and do by hand what transitions is already doing. The thing is, trying to do that is trying to replace the callback with code in 'triggers' method, and, it can work but then you loose the benefits of the library (I think), So, the doc is kinda right, you don't have to write the triggers method, what is missing IMHO, is why would you do it, and what is required to do then. Hope I did not said stupid things (that's how I understand the lib) and it helps.

EDIT: I wrote state_machine_model.trigger("transition_one") instead of state_machine_model.transition_one(). There is not implicit call when doing state_machine_model.transition_one(), that's what I meant

[campellcl]

@potens1 Thank you for clarifying, I missed that you had changed the method invocation from: state_machine_model.trigger_method_name() to state_machine_model.trigger("trigger_method_name"). My bad, I should have scrutinized your example code more carefully.

I am still going to leave this issue open for several reasons:

1. As you mentioned, the docs don't specify that this behavior will incur when you manually override the trigger methods.
   1. In my opinion the documentation should explicitly state that the user must take over manually managing state transitions in the event that the trigger methods are overridden (as you helpfully demonstrated in the code above).
   2. I was personally hoping that the library would recognize the methods were overridden, and then decorate them with the existing state transition logic automatically. I see now that this is not the case.
2. I would traditionally expect some_instance.some_method() to execute the same method as: some_instance.invoke('some_method') unless it is explicitly stated that this is not the case.
   1. As a result, I would personally consider the differences you pointed out to be a bug. I am sure the creators and maintainers of the library have a good reason for the two statements: state_machine_model.transition_one() and state_machine_model.trigger("transition_one") not being equivalent. However, It would be very helpful to have this explicitly stated in the documentation.

Regardless of this issue, thanks to all the contributors and maintainers of this library for the Pythonic and excellent finite state machine implementation! And thanks to @potens1 for your help and clarification.

[aleneum]

Hi @ccampell,

As you mentioned, the docs don't specify that this behavior will incur when you manually override the trigger methods

I will make this a bit more obvious in the readme that transitions does not override already existing methods.

I would traditionally expect some_instance.some_method() to execute the same method ...

True but the problem that we don't know whether a trigger is overridden intentionally or not. We opted for a defensive strategy where already existing methods are not touched by transitions since experience shows that indeed users occassionally have the same name for triggers and (for instance) conditions. In cases where this behaviour should be different, users can override Machine._checked_assignment:

from transitions import State, Machine

class StateMachineModel:

    state = None

    def __init__(self):
        pass

    def transition_one(self):
        print('transitioning states...')

    def transition_two(self):
        print('transitioning states...')

class OverrideMachine(Machine):

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

if __name__ == '__main__':
    states = [State(name='A'), State(name='B'), State(name='C'), State(name='D')]
    transitions = [
        {'trigger': 'transition_one', 'source': 'A', 'dest': 'B'},
        {'trigger': 'transition_two', 'source': 'B', 'dest': 'C'},
        {'trigger': 'transition_three', 'source': 'C', 'dest': 'D'}
    ]
    state_machine_model = StateMachineModel()
    state_machine = OverrideMachine(model=state_machine_model, states=states, transitions=transitions, initial=states[0])
    print('state_machine_model (current state): %s' % state_machine_model.state)
    state_machine_model.transition_one()
    print('state_machine_model (current state): %s' % state_machine_model.state)
    state_machine_model.transition_two()
    print('state_machine_model (current state): %s' % state_machine_model.state)

Output:

state_machine_model (current state): A
state_machine_model (current state): B
state_machine_model (current state): C

I also added an extended version of this to the FAQ section in the example folder.