Breaking changes to Enum support

[hperrey]

• Python State Machine version: 2.3.1 -> 2.3.2
• Python version: 3.10
• Operating System: GNU/Linux 6.5.0-42-generic

Description

The recent changes to how Enums are handled (see 6187213) broke code in existing projects for me, as the type of current_state_value as well as the capitalization of current_state.id changed.

I prefer the new current_state_value and am generally impartial to the capitalization norm, so this is not really an issue for me per-se. However, I did not expect such a change to sneak in via a minor release. Was this intended or did this slip through?

What I Did

Take a simple FSM based on Enums:

from enum import Enum
from statemachine import StateMachine
from statemachine.states import States

class TrafficState(Enum):
    """Available states to cycle through."""

    GREEN = 0
    YELLOW = 1
    RED = 2

class TrafficLightMachine(StateMachine):
    "A traffic light machine"

    states = States.from_enum(
        TrafficState, initial=TrafficState.GREEN
    )

    cycle = (
        states.GREEN.to(states.YELLOW)
        | states.YELLOW.to(states.RED)
        | states.RED.to(states.GREEN)
    )

if __name__ == '__main__':
    fsm = TrafficLightMachine()
    print(f"fsm.current_state.id: {fsm.current_state.id}, {type(fsm.current_state.id)}")
    print(f"fsm.current_state.name: {fsm.current_state.name}, {type(fsm.current_state.name)}")
    print(f"fsm.current_state_value: {fsm.current_state_value}, {type(fsm.current_state_value)}")

This yields on version 2.3.1

fsm.current_state.id: GREEN, <class 'str'>
fsm.current_state.name: Green, <class 'str'>
fsm.current_state_value: 0, <class 'int'>

and

fsm.current_state.id: green, <class 'str'>
fsm.current_state.name: Green, <class 'str'>
fsm.current_state_value: TrafficState.GREEN, <enum 'TrafficState'>

on version 2.3.2 (installed via pip).

[fgmacedo]

Hi @hperrey , how are you? Thanks for reporting this.

It was not intentional, sorry about that.

It was a huge diff between 2.3.1 and 2.3.2 and when creating the Django example project, I've refactored the example to use enum, when encountered an error, instead of fixing the example, I've changed the library 😕.

The lower case I clearly see as something that should be reverted.

The change in the value sounds like a bug fix for me. Do you think that should be reverted also?

Best!

[hperrey]

Hej @fgmacedo !

Thanks for the fast reply and explanation! I really appreciate all the work going into python-statemachine and problems like that happen, of course! :)

It is obviously your call to make, but since you asked, here are my 2 cents :

• the lower case change is probably not beneficial enough to justify the change, so I agree that reverting it would be preferable.
• I like that current_state_value (and current_state.value) are now the 'proper' Enum -- the only reason that I would not see this as a bug fix is that the previous behavior was documented iirc (see e.g. https://github.com/fgmacedo/python-statemachine/commit/6187213c4477b8d1e59fb2e69a3763ab73c5df79#diff-aef496ee2d1cf89e277f08b0270cbd9bf6ec63825c7c18b1da8b93591fc99cb0L127) and the type change is not fixing a corner case but affects any use of current_state.value. I would not mind such changes for a major version upgrade, but would not expect that a minor version requires code adjustments as long as one uses only the public API.

If you go forward with the latter change, I would suggest to highlight this in the change log -- it wasn't obvious to me where this change came from.

Just a thought: would introducing current_state.enum complicate the API more than it is worth?

[fgmacedo]

Makes sense, thanks for your arguments. I will revert both changes, and make a path towards migrating to the new behavior as default on the next major version (as it sounds like the expected default for me now).