-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update pycytominer pipelines #23
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.
Great PR, and I LOVE the formatting of how to run through all of the plates in one notebook!
I left a few comments with questions. They are mainly inquiring if some lines of code can be removed to make it more concise, specifically in Cameron's method (based on my understanding of the process). Once these are addressed, this PR is good to merge!
3.processing_features/scripts/2.pycytominer_singlecell_pipelines.py
Outdated
Show resolved
Hide resolved
@@ -7,6 +7,7 @@ dependencies: | |||
# this is restricted to Python 3.8 at this time due to other conflicts | |||
- conda-forge::python=3.8 | |||
- conda-forge::ipykernel | |||
- conda-forge::jupyterlab |
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.
Can you remind me what jupyterlab is for here?
Also, are you still having issues with getting this environment downloaded?
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.
Can you remind me what jupyterlab is for here?
I use jupyter lab for development :)
Also, are you still having issues with getting this environment downloaded?
Yes, I think the issue is the same one Dayna was having with centrosome. Do you remember the fix?
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.
I think the issues were resolved in a recent release of CellProfiler 4.2.6 in which centrosome was causing issues for all 4.0 versions (IIRC). This should be solved if you try now. Let me know if not.
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! One more question prior to merging, why are html files included? I haven't seen these included for other projects during processing.
Merging now! The html are produced by this line: https://github.com/WayScience/nf1_cellpainting_data/pull/23/files#diff-cb84f5f726165335245592b5bc3e7b535affde3de98a8a8adff909f423ba140fR13 |
Big PR to update pycytominer pipelines, please feel free to only review the nbconverted
.ipynb
files.