pytransitions / transitions

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

Support for nested states from nested Enums #404

Closed thedrow closed 4 years ago

thedrow commented 4 years ago

Consider the following Enum:

from enum import Enum, auto
class Foo(Enum):
  A = auto()
  B = auto()

class Bar(Enum):
  FOO = Foo
  C = auto()

If we were to provide Bar to a HierarchicalMachine I'd expect it to take all the items in Foo as children of the Foo state.

However this is not the case:

In[18]: from transitions.extensions import HierarchicalMachine
In[19]: HierarchicalMachine(states=Bar)
Out[19]: <transitions.extensions.nesting.HierarchicalMachine at 0x7f517ea75d90>
In[20]: machine = HierarchicalMachine(states=Bar)
In[21]: machine.states
Out[21]: 
OrderedDict([('FOO', <NestedState('FOO')@139987994034720>),
             ('C', <NestedState('C')@139987994035296>),
             ('initial', <NestedState('initial')@139987994035200>)])
In[22]: machine.states['FOO']
Out[22]: <NestedState('FOO')@139987994034720>
In[23]: machine.states['FOO'].states
Out[23]: OrderedDict()

This can be easily added if I am not wrong.

thedrow commented 4 years ago

@aleneum I closed this issue but documentation is still missing. Are you going to add it? Should I reopen?

aleneum commented 4 years ago

Documentation is one todo, yeah. Second todo is to check how transitions can be added from and to nested enums. As far as I can tell this does not work so far.

class Foo(enum.Enum):
    A = 0
    B = 1

class Bar(enum.Enum):
    FOO = Foo
    C = 2
    D = 3

m1 = HierarchicalMachine(states=Bar, initial=Bar.C)
assert m1.is_C()
m1.add_transition('go', Bar.C, Bar.D)
m1.go()  # works
assert m1.is_D()
m1.to_FOO_A()
assert m1.is_FOO_A()

m1.add_transition('go', Foo.A, Foo.B)
# m1.add_transition('go', Bar.FOO.value.A, Bar.FOO.value.B)
m1.go()
# state = _machine.get_state(state_name)
# >>> ValueError: State '['A']' is not a registered state.
thedrow commented 4 years ago

Oh that's definitely a bug. I guess we need to check if Foo is a child state of Bar.

thedrow commented 4 years ago

Actually that's also a new feature. Right now the only way to define transitions with nested states is with strings. If we document that, we can release and introduce this feature later.

aleneum commented 4 years ago

Actually that's also a new feature. Right now the only way to define transitions with nested states is with strings.

The theoretically working definition with strings (by name) would be m1.add_transition('go', 'FOO_A', 'FOO_B'). However, that does not work either because Foo.A (as m1.state) is currently considered as 'A' which is not valid. I do think the ability to define custom transitions (not just auto transitions) to enter nested enum states is a mandatory feature before nested enums can be released as it is vital for many applications. I haven't found a way so far. Do you know how this can be achieved or do you have an initial idea about how to implement that?

aleneum commented 4 years ago

So it comes down to how to resolve enums in _build_state_tree. I will probably start with a brute force search and will resolve enums by searching them in the complete state tree. That should not have much impact on users not using enums. In _build_state_list (or before) names should also be replaced by values (which is name for string states and the enum instance for enum states).

aleneum commented 4 years ago

Hi @thedrow,

do you have unpublished changes in the pipe? If not, I'd go forth and rebase next-state to clean up the commit history a bit and then merge it into master.

thedrow commented 4 years ago

No, not currently. In fact, if you can release a new version that would be awesome :smile:.

aleneum commented 4 years ago

feature has been implemented. Thanks again at @thedrow for your support. It lacks some features of 'ordinary' nested states (initial states for instance) but should be usable. closing this issue. If bugs occur, a separate issue should be opened.