statelyai / xstate-python

XState for Python
MIT License
179 stars 18 forks source link

machine.states has wrong type specification #14

Closed dimsuz closed 4 years ago

dimsuz commented 4 years ago

It seems that I've found some inconsistency.

In machine.py there's a type specification:

https://github.com/davidkpiano/xstate-python/blob/ad8f8979971411a14230ec375f4bd844b5c1f126/xstate/machine.py#L8-L14

You can see that state: List[State]. But then it is initialized like so:

self.states = self.root.states # root is a StateNode

Which, as we can see in state_node.py is a Dict[str, StateNode] and not a list of State:

https://github.com/davidkpiano/xstate-python/blob/ad8f8979971411a14230ec375f4bd844b5c1f126/xstate/state_node.py#L24

Looks like typing is off somewhere. Or did I get something wrong here? I'm not a python programmer might not know something. Also I'm not sure why no error was produced by the typechecker.

davidkpiano commented 4 years ago

Thanks for noticing this. The typing is wrong.

dimsuz commented 4 years ago

Hmm. It looks like this is still not right :)

If you look into state_node.py then you'll find that root.states is actually a Dict[str, StateNode], but machine assigns it directly to List[StateNode]:

# self.states :: List[StateNode], root.states :: Dict[str, StateNode]
 self.states = self.root.states

Looks like only dict values are needed here. (or machine should have 'Dict' too)

davidkpiano commented 4 years ago

Yep, I changed it to Dict[str, StateNode]