-
Notifications
You must be signed in to change notification settings - Fork 2
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
Linear model creation and interpretation #5
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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 LGTM! Well done.
I had several, all relatively minor, comments and suggestions. They boil down to:
- Confirming input/output paths early in notebooks
- Adding documentation in key places to help a reader understand.
There are also other comments regarding potential code upgrades and making sure alpha is adjusted for multiple testing.
1.train_models/linear_reg_plate_1_2_norm_data/nbconverted/train_model.py
Show resolved
Hide resolved
1.train_models/linear_reg_plate_1_2_norm_data/nbconverted/train_model.py
Show resolved
Hide resolved
1.train_models/linear_reg_plate_1_2_norm_data/nbconverted/train_model.py
Show resolved
Hide resolved
1.train_models/linear_reg_plate_1_2_norm_data/nbconverted/train_model.py
Show resolved
Hide resolved
1.train_models/linear_reg_plate_1_2_norm_data/nbconverted/train_model.py
Outdated
Show resolved
Hide resolved
...dels/linear_reg_plate_1_2_norm_data/feature_importances/nbconverted/seperate_compartments.py
Outdated
Show resolved
Hide resolved
...dels/linear_reg_plate_1_2_norm_data/feature_importances/nbconverted/seperate_compartments.py
Show resolved
Hide resolved
...dels/linear_reg_plate_1_2_norm_data/feature_importances/nbconverted/seperate_compartments.py
Outdated
Show resolved
Hide resolved
...dels/linear_reg_plate_1_2_norm_data/feature_importances/nbconverted/seperate_compartments.py
Show resolved
Hide resolved
...dels/linear_reg_plate_1_2_norm_data/feature_importances/nbconverted/seperate_compartments.py
Outdated
Show resolved
Hide resolved
I should also explicitly state that once a PR is approved, it can be merged, and there is no need for additional review. That being said, we expect that each scientist/engineer holds themselves to high standards and will work towards improving code and documentation. The barrier to merging a PR is primarily on the core contributor! They hold the decision power; the reviewer also has some power, but ultimately trusts the core contributor to make decisions that benefit the project readability and accuracy. Happy to continue the discussion on any particular comment, but ultimately the decisions are up to you. |
Thanks for reviewing @gwaybio, very helpful! I just had one unresolved conversation I wanted to get your opinion on |
Here I create multiple linear models that explore multiple relationships between covariates, genotypes, and morphology features. There are other analysis I'm still considering, however, any suggestions are welcome!