-
Notifications
You must be signed in to change notification settings - Fork 3
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
Updating LOIO visualization #47
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
I'm taking both Jenna and Cameron to review - please do so when you have an opportunity and the brain mood. There is no rush!! (I should clarify that we only need one review, no need for both!) |
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.
LGTM! I have a few minor comments about the figure to address but overall a very interesting visualization of the LOIO assessment 😄
Thanks again for the review @jenna-tomkinson - Given the extensive updates to this PR, would you be able to give this another look? Please focus on the figure, and I hope that it will not take too much time :)
☝️ Haha, silly me 🙃 Edit: In the most recent commit, I removed panel A, to be moved to the supplement. |
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.
Love the changes to this figure! I left one question on the figure for you to address, but overall this PR is great and ready to merge!
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.
This figure looks so much better and tells the story in a more clean and concise manner!
I only have one question:
Are these figures showing the result from the balanced or unbalanced model, with or without IC, and is this with all nuclei features (not a subset for AreaShape)?
This will probably be stated in the figure legend, but wanted to ask since I am curious what model I am looking at.
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.
Great questions! This is our standard model: Balanced, with IC, and all features. I add clarification in 8618141
Going to merge now, thanks again!
I include the following adjustments:
Note, this figure will also likely update as we continue writing and compiling the story. I consider the LOIO analysis wrapped up at this point, however 🎉