Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor app state #33

Merged
merged 83 commits into from
Apr 11, 2023
Merged

Refactor app state #33

merged 83 commits into from
Apr 11, 2023

Conversation

vgeorge
Copy link
Member

@vgeorge vgeorge commented Mar 27, 2023

While working on implementing mosaics I realized that the state of the application was really difficult to change. In the first version of Pearl we used React Context to manage the different interface flows, relying on the useEffect/useReducer to handle side effects. It turns out that this approach places the application state and side effect logic in different places, leading to a very fragmented codebase. Adding features or fixing bugs is very slow.

I started to evaluate the possibility of refactoring the code and I ended up finding XState, which seems to me to be the best approach for interfaces with a high level of complexity. Another possible refactoring approach would be to rewrite the contexts and reducers and try to organize them better, but still the logic and state would be separated. This video explains very well the difference from XState and useReducer implementations. The chart from video provides a summary of the approaches:

image

So far I managed to update the app state logic to make the prediction panel selectors work and I'm getting close rewiring a prediction run. Unfortunately this refactor will affect most functionality on the project page, but progress has been steady and the development experience with XState is great.

My goal in this PR is to get all the functionality of the prediction tab working, specifically creating a new project, opening an existing project, creating multiple AOIs and timeframes. Retrain and refinement tabs will be added later.

This is a work in progress, please let me know if you have questions/comments.

cc @geohacker @srmsoumya @LanesGood

@vgeorge
Copy link
Member Author

vgeorge commented Mar 30, 2023

@geohacker This is ready for a first review.

As I commented before, there are features that had to be disabled momentarily because of the refactor. In this PR I want implement all the features in the prediction tab, excluding AOI editing. The list of features to cover is:

  • Run live prediction on new project
  • Run live prediction on existing project
  • Switch between AOIs
  • Create new AOIs on existing projects
  • Run batch prediction on new project
  • Run batch prediction on existing project
  • Load mosaic of existing AOIs

The first four should be working already. I'll tick the list once items are reviewed and covered by tests. Please let me know your comments.

@vgeorge vgeorge marked this pull request as draft April 5, 2023 07:46
@vgeorge vgeorge marked this pull request as ready for review April 5, 2023 16:39
Copy link
Member

@geohacker geohacker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much for your work on this @vgeorge. I wasn't able to go through the entire PR — it's quite massive ⛰️. I was able to test and confirm expected workflow. Couple of regressions that are already noted that we'll address in a different PR. 🙇

@vgeorge vgeorge merged commit 7b0802d into add/select-mosaic Apr 11, 2023
@vgeorge vgeorge deleted the enhance/refactor-state branch April 11, 2023 14:00
@vgeorge vgeorge mentioned this pull request Apr 18, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants