Remove .tif/.tiff extension from list_files for MIBItiff reading in Segment_Image_Data.ipynb #281

alex-l-kong · 2020-10-16T06:22:37Z

What is the purpose of this PR?

A mini-PR that addresses and closes #279. By extension, addresses and closes #278.

How did you implement your changes

We simply add a call to extract_delimited_names in io_utils to fix this.

review-notebook-app · 2020-10-16T06:22:40Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

alex-l-kong · 2020-10-16T06:29:27Z

We still may need to address other locations where we need to remove extensions.

ngreenwald

As I mentioned in my comment on the issue, this will fail because FOVs needs to have the .tiff extension for loading mibi-tiff files.
You should change the mibi-tiff flag to be true while making these changes to make sure we fix it

ngreenwald

Rather than adding more stuff to the notebook, we could probably put this in the deepcell function itself. For example, this exact cleanup already happens if FOVs are set to None:

ark-analysis/ark/utils/deepcell_service_utils.py

Line 43 in b7d7314

fovs = io_utils.extract_delimited_names(tifs, delimiter='.')

alex-l-kong · 2020-10-22T02:48:19Z

Rather than adding more stuff to the notebook, we could probably put this in the deepcell function itself. For example, this exact cleanup already happens if FOVs are set to None:

ark-analysis/ark/utils/deepcell_service_utils.py

Line 43 in b7d7314

fovs = io_utils.extract_delimited_names(tifs, delimiter='.')

Oh wow, totally forgot about that. Let's do that instead.

alex-l-kong · 2020-10-22T03:18:10Z

I'm not a huge fan of the logic I'm using to handle the removal of the .tif extension and the verification of all the fovs in deepcell_input_dir, so would definitely appreciate a review on that.

ngreenwald

It's a bit messy, but I think it gets the job done. I think 2 things would help:

A few comments describing what's going on
A test case for input names with/without extension to make sure it handles properly

…st to tests

…into tiff_ext

ngreenwald

A few typos. We'll see if @vacuousplanet has any additional comments about how to handle the file suffixes

ark/utils/deepcell_service_utils_test.py

Co-authored-by: Noah Greenwald <[email protected]>

ackagel

So, the messiness of this code depends on how tolerant of user input we are. If we mandate fovs have their file extensions (or mandate they don't have them), then the code is much less messy. However, if we keep fovs file-extension agnostic, this 'clean-up' code it about as optimized as it can be imo.

I'm in favor of keeping file-extension agnosticism, in the hopes of a cleaner issues page 😉 .

Anyways, just one fix and I think we're gtg here

ark/utils/deepcell_service_utils.py

…verifying fovs in deepcell_input_dir

ackagel

👍

ngreenwald

Look good, two small tweaks

ark/utils/deepcell_service_utils.py

ark/utils/deepcell_service_utils_test.py

…case of .tif and .tiff files

…egment_Image_Data.ipynb (#281) * Add method to split delimited files from MIBItiff reading * Need to add the actual file exts to this * Hacky fix for MIBItiff loading * See if removing setup.py install fixes the issue we've been having * Move logic from Segment_Image_Data to deepcell_service_utils * Better (?) way of handling .tif removing? * Add comments to create_deepcell_output and add mixed fov name/file list to tests * Fix typo: existant to existent Co-authored-by: Noah Greenwald <[email protected]> * Remove extra slash from comment Co-authored-by: Noah Greenwald <[email protected]> * Remove extra check for whether .tif file exists * Fix typo and make sure we're handling both .tif and .tiff files when verifying fovs in deepcell_input_dir * Fix comments * Don't remove .zip file after creation, and make sure to handle mixed case of .tif and .tiff files * Explicitly print out error message that zip file is being overwritten Co-authored-by: Noah Greenwald <[email protected]>

Add method to split delimited files from MIBItiff reading

c7cb22b

alex-l-kong self-assigned this Oct 16, 2020

Need to add the actual file exts to this

3c73bfc

alex-l-kong requested review from ackagel and ngreenwald October 16, 2020 06:33

ngreenwald requested changes Oct 16, 2020

View reviewed changes

alex-l-kong and others added 4 commits October 16, 2020 10:49

Merge branch 'master' into tiff_ext

6193795

Merge branch 'master' into tiff_ext

d05e494

Merge branch 'master' into tiff_ext

08dd4f3

Hacky fix for MIBItiff loading

df80326

alex-l-kong requested a review from ngreenwald October 17, 2020 22:37

See if removing setup.py install fixes the issue we've been having

99a8f9b

ngreenwald requested changes Oct 21, 2020

View reviewed changes

alex-l-kong added 2 commits October 21, 2020 20:10

Move logic from Segment_Image_Data to deepcell_service_utils

03cc682

Better (?) way of handling .tif removing?

55deeb8

alex-l-kong requested a review from ngreenwald October 22, 2020 03:18

ngreenwald requested changes Oct 22, 2020

View reviewed changes

alex-l-kong and others added 3 commits October 23, 2020 15:33

Merge branch 'master' into tiff_ext

60c95ac

Add comments to create_deepcell_output and add mixed fov name/file li…

d8bbf5f

…st to tests

Merge branch 'tiff_ext' of https://github.com/angelolab/ark-analysis …

9964723

…into tiff_ext

alex-l-kong requested a review from ngreenwald October 23, 2020 23:01

ngreenwald requested changes Oct 23, 2020

View reviewed changes

ark/utils/deepcell_service_utils_test.py Outdated Show resolved Hide resolved

ark/utils/deepcell_service_utils_test.py Outdated Show resolved Hide resolved

alex-l-kong and others added 4 commits October 23, 2020 16:48

Fix typo: existant to existent

b9e837c

Co-authored-by: Noah Greenwald <[email protected]>

Remove extra slash from comment

2ef71bd

Co-authored-by: Noah Greenwald <[email protected]>

Merge branch 'master' into tiff_ext

7f9287d

Merge branch 'master' into tiff_ext

db19213

Remove extra check for whether .tif file exists

23a31f1

ackagel requested changes Oct 26, 2020

View reviewed changes

ark/utils/deepcell_service_utils.py Outdated Show resolved Hide resolved

Fix typo and make sure we're handling both .tif and .tiff files when …

b5ad33d

…verifying fovs in deepcell_input_dir

alex-l-kong requested review from ngreenwald and ackagel October 26, 2020 20:43

Fix comments

ea341f8

ngreenwald mentioned this pull request Oct 26, 2020

Warnings + time-out when sending a job to deepcell in Segment_image_data #297

Closed

ackagel approved these changes Oct 26, 2020

View reviewed changes

Merge branch 'master' into tiff_ext

354bb0d

ngreenwald requested changes Oct 27, 2020

View reviewed changes

ark/utils/deepcell_service_utils.py Outdated Show resolved Hide resolved

ark/utils/deepcell_service_utils_test.py Show resolved Hide resolved

alex-l-kong and others added 3 commits October 26, 2020 22:20

Merge branch 'master' into tiff_ext

ec7a44b

Merge branch 'master' into tiff_ext

45b0eea

Don't remove .zip file after creation, and make sure to handle mixed …

ffcec4b

…case of .tif and .tiff files

alex-l-kong requested a review from ngreenwald October 27, 2020 08:25

Explicitly print out error message that zip file is being overwritten

ce223f7

ngreenwald approved these changes Oct 27, 2020

View reviewed changes

alex-l-kong merged commit 19721f3 into master Oct 27, 2020

alex-l-kong deleted the tiff_ext branch October 27, 2020 20:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove .tif/.tiff extension from list_files for MIBItiff reading in Segment_Image_Data.ipynb #281

Remove .tif/.tiff extension from list_files for MIBItiff reading in Segment_Image_Data.ipynb #281

alex-l-kong commented Oct 16, 2020 •

edited

Loading

review-notebook-app bot commented Oct 16, 2020

alex-l-kong commented Oct 16, 2020

ngreenwald left a comment

ngreenwald left a comment

alex-l-kong commented Oct 22, 2020

alex-l-kong commented Oct 22, 2020

ngreenwald left a comment

ngreenwald left a comment

ackagel left a comment

ackagel left a comment

ngreenwald left a comment

Remove .tif/.tiff extension from list_files for MIBItiff reading in Segment_Image_Data.ipynb #281

Remove .tif/.tiff extension from list_files for MIBItiff reading in Segment_Image_Data.ipynb #281

Conversation

alex-l-kong commented Oct 16, 2020 • edited Loading

review-notebook-app bot commented Oct 16, 2020

alex-l-kong commented Oct 16, 2020

ngreenwald left a comment

Choose a reason for hiding this comment

ngreenwald left a comment

Choose a reason for hiding this comment

alex-l-kong commented Oct 22, 2020

alex-l-kong commented Oct 22, 2020

ngreenwald left a comment

Choose a reason for hiding this comment

ngreenwald left a comment

Choose a reason for hiding this comment

ackagel left a comment

Choose a reason for hiding this comment

ackagel left a comment

Choose a reason for hiding this comment

ngreenwald left a comment

Choose a reason for hiding this comment

alex-l-kong commented Oct 16, 2020 •

edited

Loading