pytransitions / transitions

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

When you add @add_state_features to a machine pickle raises errors #521

Closed jmorenobl closed 3 years ago

jmorenobl commented 3 years ago

Basically this doesn't work:

@add_state_features(MyState)
class MyMachine(Machine):
  ....

fsm = MyMachine()
with open("statechart.fsm", "wb+") as f:
   pickle.dump(fsm, f)

This is the error:

_pickle.PicklingError: Can't pickle <class 'transitions.extensions.states.add_state_features.<locals>._class_decorator.<locals>.CustomState'>: it's not found as transitions.extensions.states.add_state_features.<locals>._class_decorator.<locals>.CustomState

I think it's related to the class CustomState not being in the root of the module. I have worked around it by doing the following:

class MyMachine(Machine):
    state_cls = MyState
    ...
aleneum commented 3 years ago

Hello @jmorenobl,

as far as I know it's not possible to pickle custom/dynamically created (instances of) classes. However,

this:

@add_state_features(MyState)
class MyMachine(Machine):
   ...

is just another way of writing:

class MyMachine(Machine):
  state_cls = MyState
  ...

which -- as you pointed out -- can be pickled. If you need to combine multiple mixins, you can do it the same way:

class MyState(Error, Timeout, NestedState):
  pass

class MyMachine(HierarchicalMachine):
  state_cls = MyState
aleneum commented 3 years ago

First, thank you for pointing this limitation/drawback out. In https://github.com/pytransitions/transitions/commit/620526dba50963045cc51970fe49d7fe1eb049cd, I added a remark to the README about this. I hope this make this limitation more obvious. Right now I'd say there is not much I can do here. Especially since the workaround is not too complex/cluttered. I will close this issue for now. If you have further ideas/suggestions about how to improve @add_state_features, let me know. I will reopen the issue if necessary.

jmorenobl commented 3 years ago

Hello @aleneum ! thank you very much for your response and the great work with this library which is awesome!. I think is clear what is happening and the mention to the limitation is fair enough. The workaround is pretty easy and makes it perfectly viable to have both functionalities: adding state features through mixins and pickling.

aleneum commented 3 years ago

Thanks, @jmorenobl.

I really appreciate that you came back to comment this :).