Open auphofBSF opened 2 years ago
Thanks for this! I will be reviewing this soon, unless you want me to wait until it's ready.
Thanks for this! I will be reviewing this soon, unless you want me to wait until it's ready.
Hey @davidkpiano , a general lookaround and overview would be good, I am now getting into a cycle of commits around individual tests or sets of tests, but even these carry many supporting core implementations.
Some issues that have been tripping me up are
cyclical dependendencies the many from xstate.xxxx import yyyy
I made a large set of commits to try and solve this https://github.com/statelyai/xstate-python/pull/49/commits/c327e8aecad32b545d40f6335bbc60de32ddf809, but may need to look at overall structure more clearly
Types (or DucK Typing as is in Python) , what to do with types to support vscode intellisense some structure and verification.
To get History working against the test cases in xstate.core
, then would probably move this PR from Draft to Candidate or else one can go on for ever.
I currently have got StateNode. Transitions test cases https://github.com/statelyai/xstate-python/pull/49/commits/e2c39b03aa1812305c2991db554f620a0c24b38f and https://github.com/statelyai/xstate-python/pull/49/commits/d45493dc90f8963ecf744b1c77cb8b2e919aeb40 working and a few other test cases
There's a lot to go through here, but one thing I'm seeing that we should keep in mind is that this should not be a one-to-one reimplementation of XState v4 (JS), but rather just have the same core surface area API. That's why some of the data structures and algorithms are very different from it; there's no need to copy everything over.
What are your thoughts on that?
That is a very fuzzy line, I have selected some of the XState v4 (JS) core test cases and working back from passing those tests.
The result is that xstate-python requires adding into. Instead of re inventing it seem to make sense to use the XState v4 (JS) and keep as close to it as possible for maintaining 1 consistent logic across JS and Python.
So to define what is the Core Surface API, I have for the moment only looking for history and by association the state transitions as seen as some of the core functionality of immediate interest
That is a very fuzzy line, I have selected some of the XState v4 (JS) core test cases and working back from passing those tests.
The result is that xstate-python requires adding into. Instead of re inventing it seem to make sense to use the XState v4 (JS) and keep as close to it as possible for maintaining 1 consistent logic across JS and Python.
So to define what is the Core Surface API, I have for the moment only looking for history and by association the state transitions as seen as some of the core functionality of immediate interest
It's better to look at the v5 branch for this: https://github.com/statelyai/xstate/tree/next
That is a very fuzzy line, I have selected some of the XState v4 (JS) core test cases and working back from passing those tests. The result is that xstate-python requires adding into. Instead of re inventing it seem to make sense to use the XState v4 (JS) and keep as close to it as possible for maintaining 1 consistent logic across JS and Python. So to define what is the Core Surface API, I have for the moment only looking for history and by association the state transitions as seen as some of the core functionality of immediate interest
It's better to look at the v5 branch for this: https://github.com/statelyai/xstate/tree/next
Cool, is there a V5 change summary, and is there anywhere a vision for the xstate-python
, it is going to be difficult to not desire feature parity, just how to maintain and support, I believe the consistency of State Machine definition and its validations not to forget the great visualizations are great features. That this can be done across JS and to increasing degrees with Python is huge ! Through the code stepping and working through the features, I am getting an impressed and deeper appreciation for the work done in xstate by the main contributors 👏
latest set of commits (4th October) try to get the test_statein
--- test_should_work_to_forbid_events
working.
However the failing tests in test_statein
appear to be due to a statemachine logic failures. the first and last failure are looked at here
.
in the first failed test_should_not_transition_if_string_state_path_does_not_match_current_state_value
we end up with a state of {'a': 'a2', 'b': 'b1'}
comparing to the XState V4.25 where a1
does not transition to a2
. It appears that support for hierarchically or orthogonal related states are not handled in currently implemented transition handling.
in
(in State) not being observedtest_matching_should_be_relative_to_grandparent_no_match` transitions to foo2 , the transition has a guard but not processed as guards are not yet implemented . The guard here is
foo1: {
on: {
EVENT_DEEP: { target: 'foo2', in: 'bar.bar1' }
}
}
in
(in State) not being observedThe test_should_work_to_forbid_events
also results in the first state transitioning advancing when it should not.
in this instance the engine does not appear to be handing the transition in: { red: 'stop' }
It appears that Guards are not yet implemented in the current xstate-python
engine main_event_loop
test_should_not_transition_if_string_state_path_does_not_match_current_state_value
==== requires support for transitioning with orthogonal and hierachical state combinations
test_matching_should_be_relative_to_grandparent_no_match
requires guards
=== separate issue will be created
test_should_work_to_forbid_events
requires guards
=== separate issue will be created
I dont believe there are internal statemachine logic errors relating to these 3 fails, it is just that this functionality is not yet implemeted main_event_loop
I have done an initial compare against XState JS V4.25 where the tests pass, A further check against XState JS V5 code confirms these tests pass there as well.
Suggest these issues be raised as additional issues, to be dealt with outside of this PR, for now the unit tests affected will be supressed
The incorporation of state history fixing https://github.com/statelyai/xstate-python/issues/43 is integrated and passing all current implemented tests which includes the first 4 XStateJS history.tests. I will work my way through the remaining XStateJS history.tests . There is a bit of tidyup to also do but by large I believe it good to start reviewing for any majors with aim of finalizing this PR
Have resolved all History tests from XstateJS
that can be supported, only 3 required to be skipped. 🎉
I believe this PR is as far as I want to take it before further review. So I am removing the draft status
=========================== short test summary info ============================
SKIPPED [1] tests/test_history.py:244: Eventless Transition `Always`, not yet implemented
SKIPPED [1] tests/test_history.py:311: interpreter, not yet implemented
SKIPPED [1] tests/test_history.py:978: Transient `always` not implemented yet
SKIPPED [1] tests/test_state.py:164: Not implemented yet
SKIPPED [1] tests/test_state.py:178: Not implemented yet
SKIPPED [1] tests/test_state.py:192: Not implemented yet
SKIPPED [1] tests/test_state.py:672: Not implemented yet
SKIPPED [1] tests/test_state.py:684: Not implemented yet
SKIPPED [1] tests/test_state.py:728: Not implemented yet
SKIPPED [1] tests/test_state.py:782: Not implemented yet
SKIPPED [1] tests/test_state.py:793: Not implemented yet
SKIPPED [1] tests/test_state.py:816: Not implemented yet
SKIPPED [1] tests/test_state_in.py:230: Transition based on Orthogonal/Parallel or Hierachical relative states, not yet implemented
SKIPPED [1] tests/test_state_in.py:344: Transition Guards, not yet implemented
SKIPPED [1] tests/test_state_in.py:387: Transition Guards, not yet implemented
66 passed, 15 skipped in 37.47s
@auphofBSF This will take me some time to get through, but just for my understanding, is this trying to be a one-to-one backport of XState v4? That should not be the goal of XState Python, and if that's the case, there's a lot of things that we can omit here.
@auphofBSF This will take me some time to get through, but just for my understanding, is this trying to be a one-to-one backport of XState v4? That should not be the goal of XState Python, and if that's the case, there's a lot of things that we can omit here.
No the goal was not to be a back port of XState V4, the goal was to get history working and add tests and get a deeper understanding of the functionality of Xstate,
In using XstateV4 tests as validation, I used XStateJS V4 code as a known and validated implementation, Where I was having difficulty I could step through and understand the JS implementation and get it working in Python.
I would Like to have used XstateJS V5 but I had already implemented significant parts when you suggested I look at it and just needed to get history working. I believe I only implemented methods and attributes relevant to getting history working, but welcome the deeper look into it.
It may be worth considering refactoring xstate-python to a code base/ Interfaces closer in line with XStateJS V5 particularly from a point of view of Validation, Documentation and long term support.
If there is a desire to resolve some of the functional limitations discovered and the issues subsequently raised such as #50,#51 #52. then possibly these can be the catalyst to implement a subset of interfaces that can make use of common validation and documentation with XstateJS.
I did not want to try and resolve all functional limitations in this PR as I don't have a full vision of what Xstate-Python wants to be.
I certainly encountered difficulty with divergence from XstateJs as much of the xstate-python core implemented 2 years ago appeared to be from a different code base with resultant Interfaces quite different to what currently exists in XstateJS V4 or V5.
My particular use and understanding of how we want to use Xstate-Python is still very much emerging and exploratory but I hope I can contribute usefully to getting the great stuff I see working in JS also available for Python
THIS PR is WIP - it introduces many additional test and functions base on
xstate.core
(main) https://github.com/statelyai/xstate/commit/019f849398fc0ce0b2361e8eac12e02294fbad3bI am pushing it as a WIP PR for visibility, comment and contribution. I have in many instances pulled the complete TS module in as comments or multiline strings (They fold nicely). I have in most instances left the
xstate.core
ts
code in as comments for ease of verification.This PR is In line with needs for expanding testing, docstrings and particularly attempting to get history functioning
I am working through testcases and introducing code that is necessary to get particular test cases running with code as closely aligned to
xstate.core
Areas being worked on.
test-state
particularlytest_state_transitions
Notes on Machine config from JS snippet
I have added to Machine
__init__
to take a string such that a JS snippet of a machine config just works (exploring this use ofjs2py
further, but interestingly even a simple transition condition in JS works (NOT suggesting this should be used in PRODUCTION). This enables the direct copy and paste from JS xstate.core machine/createmachine or from the xstate-viz and avoids having to recode into pythonic dict structures, particularly useful in test case copy fromxstate.core
Example:Because of the breadth of testing updates and other issues are being touched on, my attempts may not be inline with suggestions and discussions particularly in the Issues listed below, I am however open to suggestions and alternative ways. It would be just great to get similar functionality in a python environment as is currently available in TS or JS
Following issues will have some cross over, particularly note @mr-bjerre and @nobrayner being author of many issues and contributors . and of course @davidkpiano
Other relevant issues: