pytransitions / transitions

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

Generate methods may_<trigger> for check possible transition #550

Closed artofhuman closed 2 years ago

artofhuman commented 2 years ago

Ref #549

Hi, @aleneum. I create PR with feature object.may_<trigger_name> based on your sugeestion in the issue.

coveralls commented 2 years ago

Coverage Status

Coverage increased (+0.007%) to 98.859% when pulling ad48dd7dcef7a03093b6eb950d4752417ccc4b36 on artofhuman:can-trigger into 1893a822763f9266a9691f0b285e1d77ff755082 on pytransitions:master.

aleneum commented 2 years ago

Hello @artofhuman,

thank you for the PR. This is basically how I would have done it myself. I was wondering whether we need to call prepare callbacks as well in _can_trigger before checking conditions. Prepare callbacks had been introduced since users required an opportunity to set up things before doing the actual checks.

artofhuman commented 2 years ago

Hi, @aleneum. I think we shouldn't call any callbacks in check methods to avoid side effects from the user's code.

aleneum commented 2 years ago

Yeah, I also have my concerns about this. I just remember that we introduced prepare to set up condition checks (see https://github.com/pytransitions/transitions/issues/175#issuecomment-277193912) and what happens in prepare might change the outcome of conditions. I have never used prepare that way so far but it seems to be a regular use case. For illustration's sake:

class Model:
    def __init__(self):
        self.remote_timestamp = 0
        self.local_timestamp = 0

    def prepare_cb(self):
        self.remote_timestamp = self.get_update()

    def condition_cb(self):
        return self.remote_timestamp != self.local_timestamp

# ...
m.add_transition(trigger="go", source="A", dest="B", prepare="prepare_cb", conditions="condition_cb")
m.may_go()  # >>> False
m.go() # >>> True
assert m.is_B()
broglep-work commented 2 years ago

We gave this PR a try and it does not seem to work with nested state machines

aleneum commented 2 years ago

Hello @broglep-work,

thanks for the info. FYI: I will be back working on transitions after 15th of November.

artofhuman commented 2 years ago

I've added this feature for nested states + more tests (1 fail). I've never used prepare callbacks before as well. I'll try to check this too.

aleneum commented 2 years ago

I merged your changes into the dev branch for 0.9 and added nested and async versions to the related machines. The tests pass so far. If you have some further test cases to integrated, let me know.