-
Notifications
You must be signed in to change notification settings - Fork 26
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
Added Batching Functionality to Pixel Mask Generation and FOV Saving #643
Conversation
data_utils.generate_pixel_cluster_mask
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.
Same comment applies for the cell clustering notebook
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 really nice, just some docstring updates
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 need to update the testbook stuff
ark/utils/notebooks_test_utils.py
Outdated
# cell_mask_save = """ | ||
# data_utils.save_fov_images( | ||
# pixel_fovs, | ||
# os.path.join(base_dir, pixel_output_dir), |
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.
What's the plan with this stuff?
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.
To remove it, since we don't check if the user wants to save with the batching system.
ark/utils/notebooks_test_utils.py
Outdated
# cell_mask_save = """ | ||
# data_utils.save_fov_images( | ||
# cell_fovs, | ||
# os.path.join(base_dir, cell_output_dir), |
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.
And here?
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.
To remove it, since we don't check if the user wants to save with the batching system.
If you haven't already, please read through our contributing guidelines before opening your PR
What is the purpose of this PR?
Closes #552
How did you implement your changes
Added two batching functions for the pixel and cell clustering notebooks. In addition added testbook tests.
Remaining issues
None ATM.