transitions icon indicating copy to clipboard operation
transitions copied to clipboard

GraphMachine markup should update if machine structure changes

Open drpjm opened this issue 2 years ago • 6 comments

I am experimenting with modifying HierarchicalGraphMachine for my particular use case: using remove_transition and/or removing states. I noticed that the GraphMachine family does not update its markup when structural changes occur.

Here is some example code I was messing with...

from transitions.extensions import HierarchicalGraphMachine as HGM
states = ['a','b','c']
initial = 'a'
machine = HGM(states=states, initial='a', auto_transitions=False)
machine.add_transition(trigger='t_ab', source='a',dest='b')
machine.add_transition(trigger='t_bc', source='b',dest='c')
machine.add_transition(trigger='t_ca', source='c',dest='a')
machine.get_graph().draw('machine1.png', format="png", prog='dot')

And this produces: machine1

Then I take a couple steps:

machine.t_ab()
machine.t_bc()

machine3 Now I remove a transition for fun:

machine.remove_transition(trigger='t_ca')

The transition is removed correctly (no more t_ca), but the markup still has transition as evidenced in this final graph drawing:

machine4 and by looking at the markup:

machine.markup['transitions']

[{'source': 'a', 'dest': 'b', 'trigger': 't_ab'}, {'source': 'b', 'dest': 'c', 'trigger': 't_bc'}, {'source': 'c', 'dest': 'a', 'trigger': 't_ca'}]

It could be I am doing this wrong, but I would expect that removing a transition or state should update the markup that is used to drive the graph output. If this seems reasonable, where would such a feature be implemented? Thanks!

drpjm avatar Jan 02 '23 20:01 drpjm

Hello @drpjm,

very well presented issue. Many thanks for that. Markup as well as diagrams should update when a transition is removed. Changes in 5c89a24 should address this. However, regeneration of graphs will remove the styling of previous transitions. This information is not stored anywhere and just 'applied' to a model's graph when a state changes. Consequently, your code example would end up with a graph like this:

machine2

aleneum avatar Jan 03 '23 09:01 aleneum

Great! The current state is still indicated, which is just fine. Would it make sense for there to be a method for removal of states too and that would be reflected in the markup? I did not find a remove_state function, so I have been doing something like this (based on example above):

del machine.states['a']

This operates on the OrderedDict directly. Would it make sense for there to be a wrapping function similar to remove_transition, but it safely removes the state? Or is there a better way?

drpjm avatar Jan 03 '23 14:01 drpjm

A briefly scanned the code and it seems like deleting states dynamically has not been a use case so far. But since removing transitions is supported, supporting the removal of states is not far-fetched at all. I don't see any particular reason why this should not be supported.

[...] so I have been doing something like this (based on example above): del machine.states['a']

I haven't tested in but it looks okay-ish. It will not remove convenience functions previously attached to models like "mode.is_a()" though.

aleneum avatar Jan 05 '23 12:01 aleneum

Thanks for the fix and discussion. I will play with cleanly removing states dynamically and let you know if I come up with a way to do it that would be good for the library.

drpjm avatar Jan 05 '23 18:01 drpjm

I will play with cleanly removing states dynamically and let you know if I come up with a way to do it that would be good for the library.

sounds good. adding this feature is quite straight forward I guess. remove_transition, add_state and add_model_to_state are the methods to have a look at. We usually decorate the model via setattr as far as I remember.

aleneum avatar Jan 06 '23 09:01 aleneum

In case it helps, I've found that calling machine.get_graph(force_new=True) to obtain the graph means it will pick up anything that has changed.

cc-stjm avatar Apr 18 '24 14:04 cc-stjm