New IlastikPretrainedProbability filter #1740

shanaxel42 · 2020-01-16T22:46:07Z

Creates a IlastikPretrainedProbability that takes in the path to an ilastick executable file, and ilastick project file and an Imagestack and runs ilastick headless to apply the pretrained prediction model to the Imagestack. The output of ilastik is then loaded back in as an ImageStack.

Test plan: So I tested everything locally but not sure how to write a unit test without having to install ilastick on travis...

fixes: #1499

codecov-io · 2020-01-16T23:17:40Z

Codecov Report

Merging #1740 into master will decrease coverage by 0.14%.
The diff coverage is 40.54%.

@@            Coverage Diff             @@
##           master    #1740      +/-   ##
==========================================
- Coverage   90.09%   89.94%   -0.15%     
==========================================
  Files         248      252       +4     
  Lines        9186     9579     +393     
==========================================
+ Hits         8276     8616     +340     
- Misses        910      963      +53

Impacted Files	Coverage Δ
starfish/core/image/Filter/__init__.py	`100% <100%> (ø)`	⬆️
...re/image/Filter/ilastik_pre_trained_probability.py	`38.88% <38.88%> (ø)`
starfish/data.py	`48.21% <0%> (-1.79%)`	⬇️
...fish/core/imagestack/parser/tilefetcher/_parser.py	`90.47% <0%> (ø)`
...ish/core/imagestack/parser/tilefetcher/__init__.py	`100% <0%> (ø)`
...fish/core/imagestack/test/test_from_tilefetcher.py	`100% <0%> (ø)`
...tarfish/core/test/test_multiprocessing_workflow.py	`94.87% <0%> (+0.13%)`	⬆️
starfish/core/imagestack/imagestack.py	`93.58% <0%> (+0.62%)`	⬆️
...periment/builder/test/structured_formatter/util.py	`96.7% <0%> (+1.24%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99a7f43...29edcd4. Read the comment docs.

ttung

Test plan: So I tested everything locally but not sure how to write a unit test without having to install ilastick on travis...

Is that a hard thing to do?

starfish/core/image/Filter/ilastik_pre_trained_probability.py

ttung · 2020-01-21T20:45:22Z

starfish/core/image/Filter/ilastik_pre_trained_probability.py

+    """
+
+    def __init__(self, ilastik_executable: Union[Path, str], ilastik_project: Union[Path, str]):
+        ilastik_executable = ilastik_executable \


If the assignment extends to multiple lines, I don’t see much of a point to doing x = y if cond else z (vs the traditional if statement)

ttung · 2020-01-21T20:46:38Z

starfish/core/image/Filter/ilastik_pre_trained_probability.py

+
+    def __init__(self, ilastik_executable: Union[Path, str], ilastik_project: Union[Path, str]):
+        ilastik_executable = ilastik_executable \
+            if isinstance(ilastik_executable, Path) else Path(ilastik_executable)


Actually I think Path(path) works if path is already a Path. So you might as well do:

x = Path(x)

But you may want to check that.

ttung · 2020-01-21T20:46:51Z

REQUIREMENTS.txt

@@ -5,6 +5,7 @@
 python-dateutil < 2.8.1
 click
 dataclasses==0.6
+h5py


Is this a big dependency?

nah it's not crazy just an interface between HDF5 and python

Doesn't that require sucking in a huge HDF5 library? Although we might already have that by virtue of xarray -> netcdf...

ttung · 2020-01-22T18:04:16Z

starfish/core/image/Filter/ilastik_pre_trained_probability.py

+        stack : ImageStack
+            Dapi image to be run through ilastik.
+        in_place : bool
+            N/A


How about "this parameter is ignored for this filter"?

ttung · 2020-01-22T18:05:43Z

starfish/core/image/Filter/ilastik_pre_trained_probability.py

+        np.save(dapi_file, stack.xarray.values.squeeze())
+
+        # env {} is needed to fix the weird virtualenv stuff
+        subprocess.run(


do you want subprocess.check_call? or you can pass check=True to subprocess.run

ttung · 2020-01-22T18:06:42Z

starfish/core/image/Filter/ilastik_pre_trained_probability.py

+        probability_images = h5["exported_data"][:]
+        h5.close()
+        cell_probabilities, _ = probability_images[:, :, 0], probability_images[:, :, 1]
+        cell_threshold = threshold_otsu(cell_probabilities)


wait, I thought we were going to leave the thresholding to the user.

errr good question. This was how @ambrosejcarr and @dganguli loaded in the cell_probabilities in their notebook?

ok talked with deep, will delete the thresholding bit

ttung

see comments, lgtm otherwise.

ttung · 2020-01-28T22:21:40Z

starfish/core/image/Filter/ilastik_pre_trained_probability.py

+                f"{stack.num_rounds}")
+        if stack.num_chs != 1:
+            raise ValueError(
+                f"{IlastikPretrainedProbability.__name__} given an image with more than one channel"


Suggested change

f"{IlastikPretrainedProbability.__name__} given an image with more than one channel"

f"{IlastikPretrainedProbability.__name__} given an image with more than one channel "

ttung · 2020-01-28T22:22:43Z

starfish/core/image/Filter/ilastik_pre_trained_probability.py

+                f"{stack.num_chs}")
+
+        # temp files
+        temp_dir = tempfile.TemporaryDirectory()


Suggested change

temp_dir = tempfile.TemporaryDirectory()

with tempfile.TemporaryDirectory() as temp_dir:

this will ensure cleanup.

ttung · 2020-01-28T22:23:39Z

starfish/core/image/Filter/ilastik_pre_trained_probability.py

+
+        # temp files
+        temp_dir = tempfile.TemporaryDirectory()
+        dapi_file = f"{temp_dir.name}_dapi.npy"


Suggested change

dapi_file = f"{temp_dir.name}_dapi.npy"

dapi_file = os.path.join(temp_dir.name, "dapi.npy")

ttung · 2020-01-28T22:23:59Z

starfish/core/image/Filter/ilastik_pre_trained_probability.py

+        # temp files
+        temp_dir = tempfile.TemporaryDirectory()
+        dapi_file = f"{temp_dir.name}_dapi.npy"
+        output_file = f"{temp_dir.name}_dapi_Probabilities.h5"


Suggested change

output_file = f"{temp_dir.name}_dapi_Probabilities.h5"

output_file = os.path.join(temp_dir.name, "dapi_Probabilities.h5")

This gives me a "Directory not found error"

There's something wrong then, you're not creating the temporary files in the right place.

neuromusic · 2020-01-29T18:39:55Z

🎉

#1740 added h5py but not to the pinned requirements files. That resulted in unhappy travis runs (e.g., https://travis-ci.com/spacetx/starfish/builds/146897038) Test plan: none

shanaxel42 force-pushed the saxelrod-ilastik-call-out branch 2 times, most recently from 992a4dc to 7063b29 Compare January 16, 2020 22:49

shanaxel42 force-pushed the saxelrod-from-ilastick-labeled-image branch from 8a63ef8 to c0d7f4f Compare January 16, 2020 22:51

shanaxel42 force-pushed the saxelrod-ilastik-call-out branch from 7063b29 to b35f97e Compare January 16, 2020 22:53

shanaxel42 requested a review from ttung January 21, 2020 18:08

shanaxel42 force-pushed the saxelrod-from-ilastick-labeled-image branch from c0d7f4f to 054dd28 Compare January 22, 2020 17:41

ttung requested changes Jan 22, 2020

View reviewed changes

shanaxel42 force-pushed the saxelrod-ilastik-call-out branch from b35f97e to be32e23 Compare January 22, 2020 18:44

shanaxel42 changed the base branch from saxelrod-from-ilastick-labeled-image to master January 22, 2020 18:45

shanaxel42 force-pushed the saxelrod-ilastik-call-out branch from be32e23 to 632a0aa Compare January 28, 2020 19:53

shanaxel42 requested a review from ttung January 28, 2020 19:53

ttung approved these changes Jan 28, 2020

View reviewed changes

adding method to use installed ilastik instance

29edcd4

shanaxel42 force-pushed the saxelrod-ilastik-call-out branch from 632a0aa to 29edcd4 Compare January 29, 2020 17:05

shanaxel42 merged commit b82237f into master Jan 29, 2020

ttung pushed a commit that referenced this pull request Jan 31, 2020

add h5py to pinned requirements

a48bd01

#1740 added h5py but not to the pinned requirements files. That resulted in unhappy travis runs (e.g., https://travis-ci.com/spacetx/starfish/builds/146897038) Test plan: none

ttung mentioned this pull request Jan 31, 2020

[easy] add h5py to pinned requirements #1763

Merged

ttung pushed a commit that referenced this pull request Jan 31, 2020

add h5py to pinned requirements (#1763)

8639799

#1740 added h5py but not to the pinned requirements files. That resulted in unhappy travis runs (e.g., https://travis-ci.com/spacetx/starfish/builds/146897038) Test plan: none

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New IlastikPretrainedProbability filter #1740

New IlastikPretrainedProbability filter #1740

shanaxel42 commented Jan 16, 2020 •

edited

Loading

codecov-io commented Jan 16, 2020 •

edited

Loading

ttung left a comment

ttung Jan 21, 2020

ttung Jan 21, 2020

ttung Jan 21, 2020

shanaxel42 Jan 22, 2020

ttung Jan 22, 2020

ttung Jan 22, 2020

ttung Jan 22, 2020

ttung Jan 22, 2020

shanaxel42 Jan 22, 2020

shanaxel42 Jan 28, 2020

ttung left a comment

ttung Jan 28, 2020

ttung Jan 28, 2020 •

edited

Loading

ttung Jan 28, 2020

ttung Jan 28, 2020

shanaxel42 Jan 29, 2020

ttung Jan 29, 2020

neuromusic commented Jan 29, 2020

	f"{IlastikPretrainedProbability.__name__} given an image with more than one channel"
	f"{IlastikPretrainedProbability.__name__} given an image with more than one channel "

	temp_dir = tempfile.TemporaryDirectory()
	with tempfile.TemporaryDirectory() as temp_dir:

	dapi_file = f"{temp_dir.name}_dapi.npy"
	dapi_file = os.path.join(temp_dir.name, "dapi.npy")

	output_file = f"{temp_dir.name}_dapi_Probabilities.h5"
	output_file = os.path.join(temp_dir.name, "dapi_Probabilities.h5")

New IlastikPretrainedProbability filter #1740

New IlastikPretrainedProbability filter #1740

Conversation

shanaxel42 commented Jan 16, 2020 • edited Loading

codecov-io commented Jan 16, 2020 • edited Loading

Codecov Report

ttung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ttung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ttung Jan 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neuromusic commented Jan 29, 2020

shanaxel42 commented Jan 16, 2020 •

edited

Loading

codecov-io commented Jan 16, 2020 •

edited

Loading

ttung Jan 28, 2020 •

edited

Loading