-
Notifications
You must be signed in to change notification settings - Fork 0
Suggested edits to address reviews of Chris' transient growth rate branch #49
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
Conversation
…and commented code
|
EDIT: RESOLVED ✅ thank you @pgarrison for your help @pgarrison I have struggled to figure out how to fix the |
|
cfrick13
left a comment
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.
These changes look great! Thank you so much!
Purpose
I took a stab at addressing the recommendations @chantelleleveille and I made in @cfrick13 's transient growth rate branch PR. I am making a PR into his original branch with the hopes of having handled most of the edits needed to clear bandwidth for Chris. I completed this in a separate branch so that if I misinterpreted what Chris was trying to do or he doesn't like my edits is easy to change before affecting his branch.
Edits
Testing
figure_5_s9_workflow.pysucceeds*** and figures look good except for the too-large font size on the average transient growth rate by cell cycle bin figures, as noted in my review of the original PR. I did not fix because I thought it may have been done intentionally for some reason?FigS10_workflow.pyrun_all_manuscript_workflowsonly the two workflows we anticipate failing do not succeedand post here with the results
*** Note that this does produce a bunch of runtime warnings about encountering all-NaN slices and values being set on copies of dataframes
What didn't I address here that was part of the original density PR?
There were also a huge number of new features added in the
transient_growth_f5PR and I didn't feel prepared to decide how to pate that down. That PR also adds a huge number of these to TFE which @chantelleleveille asked @cfrick13 to prune - I am leaving that up to Chris and did not do that here. There was also a little documentation/motivation that @chantelleleveille requested that I thought was out of my scope for doing this helper PR