-
Notifications
You must be signed in to change notification settings - Fork 59
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
Group tree dataprovider #902
Conversation
lindjoha
commented
Dec 30, 2021
- Move to .arrow summary data provider
- Updated file names and folder structure to best practice
a9d3e52
to
20f016c
Compare
f482aa9
to
524073e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍
Ran the plugin in and seems to work well, so just some minor comments regarding syntax.
webviz_subsurface/plugins/_group_tree/_ensemble_group_tree_data.py
Outdated
Show resolved
Hide resolved
webviz_subsurface/plugins/_group_tree/_ensemble_group_tree_data.py
Outdated
Show resolved
Hide resolved
webviz_subsurface/plugins/_group_tree/_ensemble_group_tree_data.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct from approved to request change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one minor comment, otherwise all looks good! 👍
webviz_subsurface/plugins/_group_tree/_ensemble_group_tree_data.py
Outdated
Show resolved
Hide resolved
c325a1d
to
e54ffbe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
* Typecheck error in DeckGlMap * Scroller.tsx children missing prop-type validation * add dispatch to useEffect dependency list * Moved store update on layer change from DeckGLMap to Map Co-authored-by: Shadab Khan <[email protected]>