Skip to content

Using StrEnum for states with a HierarchicalMachine induces unexpected behaviors. #695

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

Open
jbrocher opened this issue May 1, 2025 · 0 comments · May be fixed by #696
Open

Using StrEnum for states with a HierarchicalMachine induces unexpected behaviors. #695

jbrocher opened this issue May 1, 2025 · 0 comments · May be fixed by #696
Assignees
Labels

Comments

@jbrocher
Copy link

jbrocher commented May 1, 2025

Hi there, first of all I'd like to take this occasion to thank you for your awesome work with this lib. We use statecharts extensively at my company,
and this lib is a lifesaver. Been using it for years!

Describe the bug

Using StrEnum for states with a HierarchicalMachine induce the following unexpected behaviors at the moment:

  • Using the initial parameter to set the initial state causes the code to crash when resolving the initial_state
  • get_transitions does not include the expected transitions.
  • The wrong Exception is raised when using a separator in a StrEnum Member Name

These behaviors seem to be caused by string_types checks being applied before Enum checks at several points.
I listed the checks I've identified so far in "Additional Context"

Minimal working example

import enum
from transitions.extensions.nesting import HierarchicalMachine, NestedState

NestedState.separator = "__"


class States(enum.StrEnum):
    home = "a home"
    on_a_walk = "on a walk"
    walking = "currently walking"
    sitting = "currently sitting"
    currently__sitting = "currently sleeping"


class Event:
    go_home = "go_home"
    go_walk = "go_walk"
    sit = "sit"
    walk = "walk"


transitions = [
    {
        "source": States.on_a_walk,
        "dest": States.home,
        "trigger": Event.go_home,
        "before": [lambda: print("going home")],
    },
    {
        "source": States.home,
        "dest": States.on_a_walk,
        "trigger": Event.go_walk,
        "before": [lambda: print("going home")],
    },
    {"source": States.home, "dest": States.on_a_walk, "trigger": Event.go_walk},
    {
        "source": States.walking,
        "dest": States.sitting,
        "trigger": Event.sit,
        "before": [lambda: print("sitting")],
    },
    {"source": States.sitting, "dest": States.walking, "trigger": Event.walk},
]

# === Uncomment to observe crash at intialization
# machine_1 = HierarchicalMachine(
#     states=[
#         States.home,
#         {
#             "name": States.on_a_walk,
#             "children": [States.walking, States.sitting],
#             "initial": States.walking,
#         },
#     ],
#     initial=States.walking,
#     transitions=transitions,
# )


# This crashes with a KeyError("currently") instead of
# The ValueError explaining not to use the separator

# machine_2 = HierarchicalMachine(
#     states=[
#         States.currently__sleeping,
#         States.home,
#         {
#             "name": States.on_a_walk,
#             "children": [States.walking, States.sitting],
#             "initial": States.walking,
#         },
#     ],
#     transitions=transitions,
# )

machine_3 = HierarchicalMachine(
    states=[
        States.home,
        {
            "name": States.on_a_walk,
            "children": [States.walking, States.sitting],
            "initial": States.walking,
        },
    ],
    transitions=transitions,
)

# set_state works as expected
machine_3.set_state(States.walking)

# Should include a transitions for go_home and sit
# as well as to_* transitions
# assert machine_3.get_transitions("", States.walking, delegate=True) != []

# This is needed instead
target_transitions = machine_3.get_transitions(
    "", machine_3._get_enum_path(States.walking), delegate=True
)

Expected behavior

I expect StrEnum to behave as normal Enum states

Additional context

The root cause of these issues seems to be the order in which the
isinstance(<state_related_variable>, string_types) and isinstance(<state_related_variable>, Enum) are
ordered. At several places, string_types is checked first, and a member of a StrEnum is indeed as string types.

So far, I have identified this pattern at the following places:

  • in __call__*:
    def __call__(self, to_scope=None):
        if isinstance(to_scope, string_types):
            state_name = to_scope.split(self.state_cls.separator)[0]
            state = self.states[state_name]
            to_scope = (state, state.states, state.events, self.prefix_path + [state_name])
        elif isinstance(to_scope, Enum):
            state = self.states[to_scope.name]
            to_scope = (state, state.states, state.events, self.prefix_path + [to_scope.name])
        elif to_scope is None:
            if self._stack:
                to_scope = self._stack[0]
            else:
                to_scope = (self, self.states, self.events, [])
        self._next_scope = to_scope

When setting up a scoping context, if to_scope is an Enum, it will go through the first, if, to_scope.split(self.state_cls.separator)[0] will return
the value of the enum member instead of the name, and the state lookup will fail (hence the KeyError)

This happens during the recursive resolution of the initial_state. However, it does NOT happen when triggering an event, as build_state_tree use the state name,
which mean to_scope is always a string when resolving the trigger recursively in _trigger_event_nested

In get_transitions

        with self():
            source_path = [] if source == "*" \
                else source.split(self.state_cls.separator) if isinstance(source, string_types) \
                else self._get_enum_path(source) if isinstance(source, Enum) \
                else self._get_state_path(source)
            dest_path = [] if dest == "*" \
                else dest.split(self.state_cls.separator) if isinstance(dest, string_types) \
                else self._get_enum_path(dest) if isinstance(dest, Enum) \
                else self._get_state_path(dest)
            matches = self.get_nested_transitions(trigger, source_path, dest_path)
            # only consider delegations when source_path contains a nested state (len > 1)
            if delegate is False or len(source_path) < 2:
                return matches
            source_path.pop()
            while source_path:
                matches.extend(self.get_nested_transitions(trigger,
                                                           src_path=source_path,
                                                           dest_path=dest_path))
                source_path.pop()
            return matches

In the same fashion, when source is an enum, the source_path will be [<enum member value>] instead of the result of _get_enum_path because the string_types is matched first

In add_states

        for state in listify(states):
            if isinstance(state, Enum):
                if isinstance(state.value, EnumMeta):
                    state = {'name': state, 'children': state.value}
                elif isinstance(state.value, dict):
                    state = dict(name=state, **state.value)
            if isinstance(state, string_types):
                self._add_string_state(state, on_enter, on_exit, ignore, remap, **kwargs)
            elif isinstance(state, Enum):
                self._add_enum_state(state, on_enter, on_exit, ignore, remap, **kwargs)
            elif isinstance(state, dict):
                self._add_dict_state(state, ignore, remap, **kwargs)
            elif isinstance(state, NestedState):
                if state.name in self.states:
                    raise ValueError("State {0} cannot be added since it already exists.".format(state.name))
                self.states[state.name] = state
                self._init_state(state)
            elif isinstance(state, HierarchicalMachine):
                self._add_machine_states(state, remap)
            elif isinstance(state, State) and not isinstance(state, NestedState):
                raise ValueError("A passed state object must derive from NestedState! "
                                 "A default State object is not sufficient")
            else:
                raise ValueError("Cannot add state of type {0}. ".format(type(state).__name__))

And finally, when adding StrEnum states, the _add_string_state is used, which produces the wrong error message when using a separator in the name. This is
the only consequences here as far as I can tell

I've started to work on a PR to switch the order of the checks, I'll add it shortly after publishing this issue. Happy to hear your thoughts :)

Thanks again for maintaining this great project! 🎉

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

Successfully merging a pull request may close this issue.

2 participants