pytransitions / transitions

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

Nested enums now translate to nested states #405

Closed thedrow closed 4 years ago

thedrow commented 4 years ago

Fixes #404.

This needs documentation but I don't know which version this feature will released on. I'm assuming 0.9.0. Am I correct?

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-0.04%) to 98.481% when pulling 2d9e5084fc471d50009cce10eeddc3ae5a037462 on thedrow:nested-enums-to-nested-state into 070b45047a7d9c253401a39086b0f1ccc5b8dfc4 on pytransitions:master.

aleneum commented 4 years ago

That looks good to me. Some minor tweeks need to be done though (for instance here). Did you check whether the resulting Graphviz graphs look okay? Is it possible to determine the actual state and can you use auto transitions?:

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

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

m = HierarchicalMachine(states=Bar, initial=Bar.C)
m.to_FOO_A()  # does this work?
assert m.is_C() is False  # a check by state.value would return True though 
thedrow commented 4 years ago

Do we need to remove this check? https://github.com/pytransitions/transitions/blob/070b45047a7d9c253401a39086b0f1ccc5b8dfc4/transitions/extensions/nesting.py#L27-L28

thedrow commented 4 years ago

Setting the initial to Bar.C uncovered a bug in this PR that causes the test to fail because of the Graphvis extension. I fixed it now. I'll generate a graph and see if it looks correct.

thedrow commented 4 years ago

Looks right to me :) g

aleneum commented 4 years ago

Merged! Thanks again for your valuable contributions!

Setting the initial to Bar.C uncovered a bug in this PR that causes the test to fail because of the Graphvis extension.

this is comparable to #400 and should be fixed with 0a66a36. An additional test in set_node_style should not be necessary anymore. I removed it.

Sidenote: Enums with identical values cannot be distinguished. That should be at least mentioned in the documentation.

thedrow commented 4 years ago

Do we need to remove this check?

https://github.com/pytransitions/transitions/blob/070b45047a7d9c253401a39086b0f1ccc5b8dfc4/transitions/extensions/nesting.py#L27-L28

~Wait, what about this?~ Nevermind, I see your commits. But check my comments.