fgmacedo / python-statemachine

Python Finite State Machines made easy.
MIT License
858 stars 84 forks source link

Event keyword arguments are silently overwritten when particular keyword argument names are used #385

Closed owenlamont closed 1 year ago

owenlamont commented 1 year ago

Description

I tried to use a keyword argument named event on a transition action which was silently overwritten by some Python State Machine state instead.

What I Did

# Call with a https://github.com/holoviz/param Event
def ui_input_change_callback(self, event: Event) -> None:
    self.ui_statemachine.await_station_load(param_event=event)
...

# Found event is now a string, not the type I passed to the await_station_load call
def on_await_station_load(self, **kwargs) -> None:
  event: Event = kwargs["event"]

When I changed the keyword argument name from event to something else it worked fine.

I would suggest if there's certain keywords that are reserved for Python State Machine to use that it will overwrite - then Python State Machine should throw an exception if the caller tries to specify the same keyword names on a Transition/Event call.

fgmacedo commented 1 year ago

Hi @owenlamont, thanks for reporting this issue.

I understand that you encountered a problem when using a keyword argument named event in a transition action with Python State Machine. It seems that the event keyword was silently overwritten by an internal state, causing unexpected behavior.

To address this, I'd like to point out that when you specify *args or **kwargs, they will still be available to you via the built-in event_data parameter, specifically at the path event_data.trigger_data.args or event_data.trigger_data.kwargs. Therefore, even if you don't have control over the payload you're receiving (e.g., when processing events from a Kafka stream), you can still process events containing the keyword event by accessing it through the EventData dataclass.

Here's a simple code example to illustrate the use of event_data.trigger_data.kwargs:

from statemachine.event_data import EventData

def on_await_station_load(self, event_data: EventData) -> None:
    event: Event = event_data.trigger_data.kwargs["event"]

In summary, while using the event keyword might cause conflicts with Python State Machine, you can still access the data through the event_data parameter, as shown in the example. I hope this explanation clarifies your concerns. Please let me know if you have any further questions.

Does this work for you?

owenlamont commented 1 year ago

Thanks, I think I did notice that eventually. It just caught me by surprise and was a bit painful to discover what was happening in PyScript without a debugger.

I take your point though that users might not have the luxury of choosing the keyword names so I know agree they shouldn't be forbidden even when clashing.

owenlamont commented 1 year ago

I'll close this, I'm happy with the behaviour now you've explained it. Only possible action is maybe it is worth adding a small warning to the docs about this behaviour so it doesn't catch anyone else by surprise.

owenlamont commented 1 year ago

Should mention I really like your type-hinted example above, it may be contentious with some Python devs but I'd love all the examples in the docs to be type hinted where appropriate.