-
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
Improved large_nuclei splitting #244
Conversation
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 great! I just had one comment about the call to remove_small_objects
.
@@ -77,6 +78,8 @@ def compute_marker_counts(input_images, segmentation_masks, nuclear_counts=False | |||
nuc_mask = segmentation_utils.split_large_nuclei(cell_segmentation_mask=cell_mask, | |||
nuc_segmentation_mask=nuc_mask, | |||
cell_ids=unique_cell_ids) | |||
nuc_mask = remove_small_objects(ar=nuc_mask, min_size=5) |
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.
An alternative would be to put the call to remove_small_objects
inside split_large_nuclei
and add a parameter to split_large_nuclei
to control the min_size
parameter of remove_small_objects
. If min_size is not None
, then we make the additional call. This would be helpful if we plan on calling remove_small_objects
every time after split_large_nuclei
is called.
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 good. Just one nag about hardcoding.
ark/utils/segmentation_utils.py
Outdated
# only proceed if parts of the nucleus are outside of the cell | ||
if nuc_count != np.sum(nuc_mask): | ||
# only proceed if a non-negligible part of the nucleus is outside of the cell | ||
if np.sum(nuc_mask) - nuc_count > 5: |
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 5 should probably be a min_size
parameter in the split_large_nuclei
function.
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 good!
* tweak logic for large nuclei * no hardcoding Co-authored-by: alex-l-kong <31424707+alex-l-kong@users.noreply.github.com>
* tweak logic for large nuclei * no hardcoding Co-authored-by: alex-l-kong <31424707+alex-l-kong@users.noreply.github.com>
If you haven't already, please read through our contributing guidelines before opening your PR
What is the purpose of this PR?
Additional tweaks to the nuclei split logic. One issue that came up was if the nuclear mask was just a tiny bit bigger than the cell mask. For example, one pixel larger on one side. This resulted in potentially a single pixel being cut from the mask, relabeled, and given a new ID. This creates tons of new, tiny nuclear objects, which we don't want.
How did you implement your changes