Generalized masking #1092

camisowers · 2023-12-05T02:07:06Z

If you haven't already, please read through our contributing guidelines before opening your PR

What is the purpose of this PR?

Closes #1091.

How did you implement your changes

Add two user functions, generate_signal_masks() and generate_cell_masks() for either signal based masking or cell based masking.

Remaining issues

create masking notebook

review-notebook-app · 2023-12-05T18:42:07Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

camisowers · 2023-12-05T18:58:51Z

I still need to write the notebook tests, but would love any preliminary input on the masking functionalities you have helped write or seen lab members use. @alex-l-kong @srivarra

srivarra

It seems EzSeg already has some of this functionality in one form or another. We could figure out how to use it here, or we could make changes in ezseg to account for this workflow. Thoughts?

src/ark/utils/masking_utils.py

alex-l-kong

@srivarra pretty much covered it.

camisowers · 2024-04-16T21:43:21Z

Updated to use the already built ezseg functions. Decided to forego the notebook tests, since it's just a utility notebook with only top-level function calls that are already tested.

alex-l-kong

A few comments about what should be visible or not, but otherwise looks great.

src/ark/utils/masking_utils.py

alex-l-kong · 2024-04-17T22:24:23Z

src/ark/utils/masking_utils.py

+    return cell_mask
+
+
+def generate_cell_masks(seg_dir, mask_dir, cell_table, cell_types, cluster_col, mask_name,


I would add a few defaults here:

cell_types: default to None, in which case include all cell types in create_cell_masks

cluster_col: should be ark.settings.cell_type. In that way you may not have to explicitly specify it in the notebook.

The cluster_col default is a great idea! For cell_types, it wouldn't really make sense to mask all cell types in the image (it would just be a blurred version of the segmentation masks), so forcing the user to specify a list is robably the way to go.

src/ark/utils/masking_utils.py

alex-l-kong · 2024-04-17T22:30:04Z

templates/Generalized_Masking.ipynb

@@ -0,0 +1,155 @@
+{


Personally I prefer leaving sigma, min_mask_size, and max_hole_size as hidden default arguments inside generate_signal_masks. If you expect the user will change these, then I'd specify them as customizable variables in the cell above.

Reply via ReviewNB

The defaults set here are currently based only on what Noah used for the TONIC data, so I don't want users to think the masks generated with these are the only output possible. I'll add them to the description and set them in the cell above.

alex-l-kong · 2024-04-17T22:30:04Z

templates/Generalized_Masking.ipynb

@@ -0,0 +1,155 @@
+{


Piggybacking off of a comment from ark.utils.masking_utils.py, I'd prefer leaving cluster_col as a hidden default argument in generate_cell_masks.

Reply via ReviewNB

Similar to the spatial scripts, this cell masking would often be used for a more broad cell type grouping specified by the user previously, so rarely would we use the 'cell_meta_cluster' column. It's just set here as a example!

templates/Generalized_Masking.ipynb

srivarra

Looks pretty good, just a couple of clarifications.

srivarra · 2024-04-18T17:37:18Z

src/ark/segmentation/ez_seg/ez_object_segmentation.py

@@ -154,7 +154,9 @@ def _create_object_mask(
    misc_utils.verify_in_list(object_shape_type=[object_shape_type], object_shape_options=["blob", "projection"])

    # Copy the input image, and get its shape
-    img2mask: np.ndarray = input_image.copy().to_numpy()
+    img2mask: np.ndarray = input_image.copy()


In what instance would this not be a numpy array? If it's an xarray, all normal numpy ops and functions which make use of numpy ops will work, so we wouldn't need to convert it.

Where I'm implementing it in masking_utils, img2mask is already an np.array, so I get an AttributeError: 'numpy.ndarray' object has no attribute 'to_numpy'.

srivarra · 2024-04-18T17:53:50Z

src/ark/segmentation/ez_seg/composites.py

+            "composite_directory": composite_directory,
+            "composite_name": composite_name,
+        }
+        log_creator(variables_to_log, log_dir, f"{composite_name}_composite_log.txt")


Why do we not want to return the composite images here as well? For example what if the user passes in a path for log_dir but there isn't a path for the composite_directory. It wouldn't return the dictionary nor save any of the composite files.

The ezseg functions only save the composites, they don't return them so I would have to change the ezseg notebook to handle a return (not a big deal, but unnecessary when it's not even used). Since the only case we would need the composites returned is for the internal function call inside generate_signal_masks(), I didn't think it was needed.

src/ark/segmentation/ez_seg/composites.py

srivarra

Looks good!

…thlib.Path if of type str

camisowers added 2 commits December 4, 2023 11:46

init

197cd84

tests

9f4325c

camisowers added the enhancement New feature or request label Dec 5, 2023

camisowers self-assigned this Dec 5, 2023

notebook

2575a21

camisowers requested review from alex-l-kong and srivarra December 5, 2023 18:57

camisowers marked this pull request as ready for review December 5, 2023 18:59

srivarra requested changes Dec 5, 2023

View reviewed changes

src/ark/utils/masking_utils.py Outdated Show resolved Hide resolved

src/ark/utils/masking_utils.py Outdated Show resolved Hide resolved

alex-l-kong reviewed Dec 6, 2023

View reviewed changes

camisowers and others added 11 commits December 13, 2023 21:15

Merge branch 'main' into signal_masking

f288cb6

use ez_seg.composite.composite_builder

348caf5

Merge branch 'main' into signal_masking

8b8566d

fix composite_dir if statement

3a8a908

Merge remote-tracking branch 'origin/signal_masking' into signal_masking

15ae88a

use _create_object_mask instead of create_mask

02032ed

remove squidpy requirement

3771b07

remove dask dependecy

a9ecb44

pycodestyle

f55911e

notebook updates

4b1f9c8

notebook tweaks

5b024c6

camisowers requested review from alex-l-kong and srivarra April 16, 2024 21:42

alex-l-kong requested changes Apr 17, 2024

View reviewed changes

alex-l-kong reviewed Apr 17, 2024

View reviewed changes

feedback tweaks

a1fc940

srivarra requested changes Apr 18, 2024

View reviewed changes

feedback tweaks II

c5b0e67

camisowers requested review from alex-l-kong and srivarra April 19, 2024 19:36

srivarra approved these changes Apr 19, 2024

View reviewed changes

alex-l-kong approved these changes Apr 24, 2024

View reviewed changes

camisowers added this pull request to the merge queue Apr 24, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 24, 2024

camisowers and others added 5 commits April 24, 2024 11:47

pin torchdata

04b8412

pin pytorch

1572f9e

remove pytorch pin

34a32e8

Test with torch==2.2.0, this worked with Kausi

3c48aa4

Handle case where composite_directory needs explicit conversion to pa…

61e0b85

…thlib.Path if of type str

alex-l-kong added this pull request to the merge queue Apr 25, 2024

Merged via the queue into main with commit c877314 Apr 25, 2024
18 checks passed

alex-l-kong deleted the signal_masking branch April 25, 2024 21:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generalized masking #1092

Generalized masking #1092

camisowers commented Dec 5, 2023 •

edited

Loading

review-notebook-app bot commented Dec 5, 2023

camisowers commented Dec 5, 2023

srivarra left a comment

alex-l-kong left a comment

camisowers commented Apr 16, 2024 •

edited

Loading

alex-l-kong left a comment

alex-l-kong Apr 17, 2024

camisowers Apr 18, 2024

alex-l-kong Apr 17, 2024 •

edited

Loading

camisowers Apr 18, 2024 •

edited

Loading

alex-l-kong Apr 17, 2024 •

edited

Loading

camisowers Apr 18, 2024

srivarra left a comment

srivarra Apr 18, 2024

camisowers Apr 18, 2024

srivarra Apr 18, 2024

camisowers Apr 18, 2024

srivarra left a comment

		return cell_mask


		def generate_cell_masks(seg_dir, mask_dir, cell_table, cell_types, cluster_col, mask_name,

Generalized masking #1092

Generalized masking #1092

Conversation

camisowers commented Dec 5, 2023 • edited Loading

review-notebook-app bot commented Dec 5, 2023

camisowers commented Dec 5, 2023

srivarra left a comment

Choose a reason for hiding this comment

alex-l-kong left a comment

Choose a reason for hiding this comment

camisowers commented Apr 16, 2024 • edited Loading

alex-l-kong left a comment

Choose a reason for hiding this comment

alex-l-kong Apr 17, 2024

Choose a reason for hiding this comment

camisowers Apr 18, 2024

Choose a reason for hiding this comment

alex-l-kong Apr 17, 2024 • edited Loading

Choose a reason for hiding this comment

camisowers Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

alex-l-kong Apr 17, 2024 • edited Loading

Choose a reason for hiding this comment

camisowers Apr 18, 2024

Choose a reason for hiding this comment

srivarra left a comment

Choose a reason for hiding this comment

srivarra Apr 18, 2024

Choose a reason for hiding this comment

camisowers Apr 18, 2024

Choose a reason for hiding this comment

srivarra Apr 18, 2024

Choose a reason for hiding this comment

camisowers Apr 18, 2024

Choose a reason for hiding this comment

srivarra left a comment

Choose a reason for hiding this comment

camisowers commented Dec 5, 2023 •

edited

Loading

camisowers commented Apr 16, 2024 •

edited

Loading

alex-l-kong Apr 17, 2024 •

edited

Loading

camisowers Apr 18, 2024 •

edited

Loading

alex-l-kong Apr 17, 2024 •

edited

Loading