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

A single condition for multiple transitions #269

Closed
parthsharma1996 opened this issue Dec 1, 2017 · 12 comments
Closed

A single condition for multiple transitions #269

parthsharma1996 opened this issue Dec 1, 2017 · 12 comments

Comments

@parthsharma1996
Copy link

I was not able to find a direct way to have a single function act as a condition for multiple transitions from a state, so that different transitions are triggered based on different values of the function func() . For example:-

Let's say I'm in the state A. I want to transition:-

  1. to B when func() returns 1
  2. to C when func() returns 2
  3. to D when func() returns 3
    ....... and so on

It seems that the conditions functions only works on True and False values however.

A way to implement this that I can think of in the current functionality would be to create n wrapper functions for n transitions and have them compare the results of func() to their respective numbers. However, this is a cumbersome process.

Can this be added as a feature?

@aleneum
Copy link
Member

aleneum commented Dec 4, 2017

Hello @parthsharma1996,

even though there is no direct way of declaring a transition based on a function result, it can be achieved with a list of transitions with the same trigger and varying conditions:

transitions = [{'trigger': 'call', 'source': 'A', 'dest': 'B', 'prepare':'func', 'conditions': 'result_is_1'},
               {'trigger': 'call', 'source': 'A', 'dest': 'C', 'conditions': 'result_is_2'},
               {'trigger': 'call', 'source': 'A', 'dest': 'D'}]

prepare is called once before the transitions are evaluated. We can omit the last condition check in case func is guaranteed to return 1, 2 or 3.

If I understand correctly, you are aware of this.
I agree that if your design contains a lot of these conditional transitions, defining those transitions manually probably clutters your configuration.

A way to specify such a transition could look like this:

transition =  dict(trigger='shuffle', source='A', dest=({1: 'B', 2: 'C', 3: 'D'}), depends_on='func')

You could then override Machine.add_transitions to implement a 'wrapper':

from transitions import Machine
from transitions.core import listify

class ExtendedMachine(Machine):

    def add_transition(self, trigger, source, dest, conditions=None,
                       unless=None, before=None, after=None, prepare=None, **kwargs):

        if isinstance(dest, dict):
            prepare = listify(prepare)
            conditions = listify(conditions)
            for idx, k in enumerate(dest):
                # add depends_on as a preparation call for the first transition
                prep = [kwargs.pop('depends_on')] + prepare if idx == 0 else prepare
                # add result check as the first condition for a transition
                self.add_transition(trigger, source, dest[k], [lambda: self._result == k] + conditions,
                                    unless, before, after, prep, **kwargs)
        else:
            super(ExtendedMachine, self).add_transition(trigger, source, dest, conditions,
                                                         unless, before, after, prepare, **kwargs)

Or you can subclass Transition itself to make its dest property return a different value based on depends_on result:

from transitions import Machine, Transition
from six import string_types

class DependingTransition(Transition):

    def __init__(self, source, dest, conditions=None, unless=None, before=None,
                 after=None, prepare=None, **kwargs):

        self._result = self._dest = None
        super(DependingTransition, self).__init__(source, dest, conditions, unless, before, after, prepare)
        if isinstance(dest, dict):
            try:
                self._func = kwargs.pop('depends_on')
            except KeyError:
                raise AttributeError("A multi-destination transition requires a 'depends_on'")
        else:
            # use base version in case transition does not need special handling
            self.execute = super(DependingTransition, self).execute

    def execute(self, event_data):
        func = getattr(event_data.model, self._func) if isinstance(self._func, string_types) \
               else self._func
        self._result = func()
        super(DependingTransition, self).execute(event_data)

    @property
    def dest(self):
        return self._dest[self._result] if self._result is not None else self._dest

    @dest.setter
    def dest(self, value):
        self._dest = value

# subclass Machine to use DependingTransition instead of standard Transition
class DependingMachine(Machine):
    transition_cls = DependingTransition

Both approaches should be considered as drafts. The second version might keep your transition count a bit lower but will probably cause incomplete dot representations of the state machine. The first version does not consider the case of a not set depends_on keyword. Additionally, the first version requires some way of how the return value of func is stored into self._result or probably better self.__result to prevent accidental overrides.

Can this be added as a feature?

This depends on the feedback of the user base. I see a value in this addition. The integration will very likely increase the complexity of the core code though which we like to avoid.

Feel free to add you ideas/suggestions

@tyarkoni
Copy link
Member

tyarkoni commented Dec 4, 2017

Yeah I agree there's a place for this, but barring demand from other users, I don't think it justifies the increase in code complexity.

I wonder if the way to handle this is to create a new subfolder of notebooks called "patterns" or something like that under examples, and then every time something like this comes up, we can create a new notebook that illustrates the usage. We can then put a README in there with brief summaries of all of these different usage patterns that don't quite rise to the level of core features or extensions.

Probably worth a sweep through old issues at some point too, as I think there have been a bunch of similar issues before that we've closed without implementing, but might be hard to find by searching.

@tyarkoni
Copy link
Member

tyarkoni commented Dec 4, 2017

Actually, the FAQ notebook looks it's already the right place for that--except maybe we should make it a folder and put each question and answer in its own notebook.

@aleneum
Copy link
Member

aleneum commented Dec 4, 2017

the FAQ notebook looks it's already the right place for that--except maybe we should make it a folder

sounds good. Notebooks are also indexed and therefore searchable.

@parthsharma1996
Copy link
Author

parthsharma1996 commented Dec 5, 2017

Thanks for the example codes @aleneum . I will try and use one of the two approaches. This certainly would be a useful feature for my use-case i.e. Building a chatbot

@parthsharma1996
Copy link
Author

@aleneum Also some users mentioned storing FSM configs as YAML files. I'm not entirely sure how to store the python functions in YAML. Could you provide an example of that, if you're aware of this?

@aleneum
Copy link
Member

aleneum commented Dec 7, 2017

This sounds like something very suited for Stackoverflow. I quote myself from the past:

This has several advantages:

  • Your question gains higher visibility since most developers look for help there
  • The targeted community is larger; Some people will even help you to formulate a good question
  • People get 'rewarded' with 'reputation' to help you. You also gain reputation in case this questions pops up more frequently. It's a win-win situation.

@parthsharma1996
Copy link
Author

@Alaneum I posted this on StackOverflow here with relevant tags, however I didn't receive any replies. What would you suggest I do?

@aleneum aleneum mentioned this issue Jan 3, 2018
3 tasks
@aleneum
Copy link
Member

aleneum commented Jan 3, 2018

I posted a recommendation including a link to a blog post where serialisation and JSON injection is presented. This should be also applicable for YAML. The above mentioned condition based transition feature will not be integrated into transitions (yet). However, the above mentioned approach(es) will be documented in the example section.

@amitabhn
Copy link

amitabhn commented May 6, 2019

I think this would be a useful feature to have in order to simplify the transitions configuration.
In my particular use case of an event driven embedded system, the destination state of a transition depends on a combination of physical states of multiple buttons.
Also, given that the transitions configuration list forms the core logic of the state machine, I would want it to be as concise, readable and easily understandable as possible and not make it too verbose. So instead of having to define multiple transition dicts for the same trigger and source state for multiple conditions, it would be better to keep it centralized at one place.

@sergei3000
Copy link

+1 for this feature. It would be great to be able to define just one element in the list of transitions instead of defining multiple transitions. This would make my code base more readable, which is always great.

@AmirAktify
Copy link

+1 for this feature for me too.

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

No branches or pull requests

6 participants