Deepcell task #199

omerbt · 2020-08-25T08:02:32Z

This branch includes a new method for automating the process of uploading files to DeepCell, downloading and extracting the output .tif image.

Implementation is based on relevant modules from https://github.com/vanvalenlab/kiosk-client.

According to the current implementation of the kiosk-client, the DeepCell output will be downloaded to a predetermined folder (/downloads). Ideally, the path should be a parameter.

# Conflicts: # requirements.txt

ngreenwald

Hey Omer, great start! The build is failing because of a formatting issue, you can just add a blank line at the end of the file and you should be set.

Could you please update the description of the PR with the indicated information?

# Conflicts: # ark/utils/deepcell_service_utils.py

omerbt · 2020-08-25T18:45:41Z

Hey, thanks. I fixed the indentation so it be compliant with PEP and added a short (is it enough?) description .

ngreenwald

Awesome, just a quick question about the requirements.txt file, which looks like it has some unused functions in it.

Given that you're not actually in our lab, no pressure to do any more than this, it will already be super helpful.

If you're interested in working some more on this, let me know. I think some nice additions would be creating a top-level function that creates a directory for each Point, calls this function with that directory as the output, and then renames the output appropriately. This would avoid having to get the last modified file. It would also be good to add some testing to make sure this works as expected. If you have other stuff you're working on, no worries, we can handle that part. But if you're interested in fleshing this out a bit more, let me know.

Thanks!

ngreenwald · 2020-08-25T20:37:47Z

requirements.txt

+certifi
+pyOpenSSL
+service_identity
+git+https://github.com/vanvalenlab/kiosk-client@master


I think the client is pip installable: can you try adding kiosk_client to the requirements.txt file?

It doesn't look like certifi, pyOpenSSL, or service_identify are imported anywhere, is there a reason these are required?

I'd be happy to add more methods.
regarding the requirements - they are required in order to be able to upload files using twisted and for the tls certificate (even tough they are not explicitly called in the method). I'm changing the git dependency to pip installation.
Thanks!

ngreenwald · 2020-08-25T22:04:06Z

Okay great! I opened an issue in the kiosk-client to see if there's a reason Will set it up this way. If not, I think it would be pretty easy to submit a PR allowing us to specify an output path.

Once we get an answer on that, we'll know how to move forward. I think we'll want to create a function like create_deepcell_output(). This function should handle all of the necessary data manipulation for running deepcell. It should expect a path to a folder of already formatted images, call the helper function you've already written on each of those images, and then format the output of the helper function appropriately.

Once that's set up, we can update the jupyter notebook to use this new function

ngreenwald · 2020-08-26T00:45:53Z

Okay, so Will said the reason they never added that option was just because they didn't get to it. I think a PR to address that would be great. If you'd like to take that on, let me know, otherwise Alex or Adam can put that on their todo list.

What that means is that the high level function can expect the files to be named appropriately. This will simplify things further.

Let me know what stuff you'd like to get started on

omerbt · 2020-08-26T05:30:49Z

Great, so I can start with that PR and then move on to create_deepcell_output()

ngreenwald · 2020-08-26T05:46:53Z

Perfect, sounds great. If you don't have write access to kiosk-client you fork the repo and submit your PR from your branch, or ask Will to give you write access.

If you run into any issues feel free to leave a comment on the issue I opened in that repo and Will or I can respond.

omerbt · 2020-08-26T06:34:15Z

Great thanks!

alex-l-kong · 2020-09-25T22:18:21Z

What's the need for RTD to have access to all of the random dependencies in the project?

It's an annoying part of working with RTD and specifically, building documentation from docstrings. When snaking through the documentation, I think autodoc, which builds the documentation, first looks through the import statements and complains immediately if it can't import it properly (which can only be guaranteed by mocking it up in autodoc_mock_imports). Part of the reason I think it does this is to properly handle a documentation arg-list value or return type of, say, np.ndarray or pd.DataFrame. This is especially the case if you, for example, use something like intersphinx_mapping to directly link to NumPy or Pandas documentation, which we do.

So in Omer's case, when it sees a library such as kiosk_client, I think what's happening is that it tries to import it first so it can properly build any documentation that potentially includes an argument or return type of kiosk_client in it. If it can't import it, it fails before it tries to build potentially bad documentation with potential non-existent references to kiosk_client.

alex-l-kong · 2020-09-25T22:29:14Z

@ngreenwald I'm guessing one of your concerns is that developers have to know to add to docs/conf.py, which requires them to have had some experience working with Sphinx/RTD in the past.

I've been looking into a way to somehow tell RTD to do a pip install -r requirements.txt on their end so they don't have to rely on autodoc_mock_imports, but my leads have come up dry for now. It's for another PR, for sure.

ngreenwald · 2020-09-25T23:21:35Z

My main concern is that kiosk-client has a ton of dependencies that we don't actually care about. So mocking it directly sounds like the best approach since there isn't anything in the documentation that relies on us referencing a class or something from there.

Thanks!

ngreenwald

I talked to Will, it turns out twisted is a requirement of treq, which is a requirement for kiosk-client.

Can you just double check (by commenting out one at a time and rebuilding the environment) that each of the current requirements are actually needed? If so, we can of course add them, but I'd like to avoid adding unnecessary requirements if possible if they're getting installed via the kiosk.

alex-l-kong

This looks awesome Omer! I had some brief comments about the documentation, testing, and requirements.txt.

alex-l-kong · 2020-09-26T17:21:06Z

requirements.txt

+Twisted~=20.3.0
+pyOpenSSL
+service_identity


Realize this is basically a piggyback off of Noah's comment now, sorry! I wonder if simply including kiosk-client will do the trick. Looking at setup.py of the kiosk-client repo, it looks like it's configured to install treq on a pip install, meaning that if all goes well, we might be able to remove at least twisted, if not also pyOpenSSL and service_identity (assuming treq installs at least twisted as well).

Yes it does! I only checked the requirements file itself (of kiosk-client), but as you and @ngreenwald said, kiosk-client installs all the necessary modules. thanks

alex-l-kong · 2020-09-26T17:26:04Z

ark/utils/deepcell_service_utils.py

+                warnings.warn(f'Deep Cell output file was not found for {point}.')
+
+
+def run_deepcell_task(input_dir, output_dir, host='https://deepcell.org',


To clarify, we don't need a test function for this one?

I couldn't think of any testing that would actually be useful. We'd have to mock basically the entire function, and at that point I'm not sure what we're actually testing. Open to suggestions if you think there's functionality worth testing

Ok, I didn't think there was any additional testing we needed for this since its basically a driver function. Don't think we need to add a test function for this.

alex-l-kong · 2020-09-26T17:26:47Z

ark/utils/deepcell_service_utils.py

+                Default: 'multiplex'
+        """
+
+    # more configuration parameters can be set. https://github.com/vanvalenlab/kiosk-client


You may want to put this in the docstring description (before the Args list) so people can reference it on ReadTheDocs.

I updated it in the last commit

ngreenwald · 2020-09-27T17:55:31Z

Hey Omer, this looks great! I'll wait until Adam takes a look before approving in case he has any additional suggestions.

If you'd like to keep contributing to the repository, I've created an issue outlining the next steps for a subsequent PR. If you've got other stuff to work on, no worries at all, one of us can tackle it. Just let me know what works for you!

Here's the issue: #243

alex-l-kong · 2020-09-27T18:34:13Z

Ditto on Noah's comment. It looks good to me, let's see what Adam has to say.

ackagel

Looks fantastic! Just one suggestion for the points argument in create_deepcell_output.

ackagel · 2020-09-28T00:42:02Z

ark/utils/deepcell_service_utils.py

+            points (list):
+                List of points in preprocessing pipeline


I think an option to set points=None so that all .tif files in the input directory can be read in would be pretty useful. We have some tools written to help with this. If you import ark.utils.io_utils, you could use something like:

if points is None: tifs = io_utils.list_files(deepcell_input_dir, substrs='.tif') points = io_utils.extract_delimited_names(tifs, delimiter='.')

That should plug directly into the code you've already written, assuming I didn't make any typos, etc.

added in the last commit

+ testing

omerbt · 2020-09-28T15:29:58Z

@ngreenwald thanks! sure, I'll take it.

alex-l-kong

Hi Omer, this looks great! I had one comment about your docstring for create_deepcell_output.

alex-l-kong · 2020-09-28T16:41:41Z

ark/utils/deepcell_service_utils.py

+            points (list):
+                List of points in preprocessing pipeline. if None, all .tif files
+                in deepcell_input_dir will be considered as input points.
+            deepcell_input_dir (str):
+                Location of preprocessed files (assume deepcell_input_dir contains <point>.tif
+                for each point in points list)
+            suffix (str):
+                Suffix for DeepCell output filename. e.g. for pointX, DeepCell output
+                should be <pointX>+suffix.tif. Default: '_feature_0'
+            deepcell_output_dir (str):
+                Location to save DeepCell output (as .tif)
+            host (str):
+                Hostname and port for the kiosk-frontend API server
+                Default: 'https://deepcell.org'
+            job_type (str):
+                Name of job workflow (multiplex, segmentation, tracking)
+                Default: 'multiplex'


You should put your args list in the order you specified your parameters in the function (so, deepcell_input_dir, deepcell_output_dir, points, etc.).

Updated in the last commit

alex-l-kong

Awesome, I think it's good to go!

ngreenwald

Great!

ackagel

Looks great!

* Use kiosk-client modules for automated DeepCell tasks * Use kiosk-client modules for automated DeepCell tasks * Use kiosk-client modules for automated DeepCell tasks * Update deepcell_service_utils.py * Use kiosk-client modules for automated DeepCell tasks * Use kiosk-client modules for automated DeepCell tasks * Use kiosk-client modules for automated DeepCell tasks * Update deepcell_service_utils.py * Use kiosk-client modules for automated DeepCell tasks * Use kiosk-client modules for automated DeepCell tasks * Update deepcell_service_utils.py * Use kiosk-client modules for automated DeepCell tasks * additional requirements for twister certificates * install kiosk-client using pip * create_deepcell_output * create_deepcell_output * Update requirements.txt Co-authored-by: Noah Greenwald <noahfgreenwald@gmail.com> * create_deepcell_output * changing back to 'kiosk-client' * basic testing framework * modified requirements.txt file * fixed version issue * input validation + unit tests * rerun build * docstrings formatting * docstrings formatting * requirements.txt * requirements.txt * requirements.txt * add dependencies to conf.py * test multiple .tif * remove unnecessary modules * remove unnecessary modules * make points an optional parameter + testing * Docstring parameters reorder Co-authored-by: Noah Greenwald <noahfgreenwald@gmail.com>

omerbt and others added 4 commits August 25, 2020 02:36

Use kiosk-client modules for automated DeepCell tasks

aea2266

Use kiosk-client modules for automated DeepCell tasks

b74ca52

Use kiosk-client modules for automated DeepCell tasks

7b99c21

Update deepcell_service_utils.py

30e7ea3

omerbt requested a review from ngreenwald August 25, 2020 08:07

omerbt and others added 7 commits August 25, 2020 16:41

Use kiosk-client modules for automated DeepCell tasks

3afd201

Use kiosk-client modules for automated DeepCell tasks

07ea943

Use kiosk-client modules for automated DeepCell tasks

5939c84

Update deepcell_service_utils.py

7b29d99

Use kiosk-client modules for automated DeepCell tasks

42e79b3

Use kiosk-client modules for automated DeepCell tasks

e0bd133

Merge remote-tracking branch 'origin/deepcell-task' into deepcell-task

027e5e1

# Conflicts: # requirements.txt

omerbt marked this pull request as draft August 25, 2020 14:03

ngreenwald requested changes Aug 25, 2020

View reviewed changes

omerbt and others added 4 commits August 25, 2020 18:57

Update deepcell_service_utils.py

95ff027

Use kiosk-client modules for automated DeepCell tasks

aede42f

Merge remote-tracking branch 'origin/deepcell-task' into deepcell-task

bf34132

# Conflicts: # ark/utils/deepcell_service_utils.py

additional requirements for twister certificates

c97b7b4

omerbt marked this pull request as ready for review August 25, 2020 18:44

ngreenwald requested changes Aug 25, 2020

View reviewed changes

install kiosk-client using pip

3e8e9b6

omerbt added 2 commits September 12, 2020 21:42

create_deepcell_output

1d91fb3

create_deepcell_output

80d4b8b

ngreenwald self-requested a review September 12, 2020 19:45

omerbt added 2 commits September 26, 2020 13:36

add dependencies to conf.py

7000e0a

test multiple .tif

8859a73

omerbt requested a review from ngreenwald September 26, 2020 11:08

ngreenwald requested changes Sep 26, 2020

View reviewed changes

ngreenwald requested review from alex-l-kong and ackagel September 26, 2020 15:32

alex-l-kong requested changes Sep 26, 2020

View reviewed changes

omerbt added 2 commits September 27, 2020 12:23

remove unnecessary modules

e8387f6

remove unnecessary modules

2ad9ebe

omerbt requested review from alex-l-kong and ngreenwald September 27, 2020 09:43

ackagel reviewed Sep 28, 2020

View reviewed changes

make points an optional parameter

0c9523d

+ testing

alex-l-kong reviewed Sep 28, 2020

View reviewed changes

Docstring parameters reorder

0c976c5

alex-l-kong self-requested a review September 28, 2020 17:30

alex-l-kong approved these changes Sep 28, 2020

View reviewed changes

ngreenwald approved these changes Sep 28, 2020

View reviewed changes

Merge branch 'master' into deepcell-task

fafe04b

ackagel approved these changes Sep 28, 2020

View reviewed changes

ngreenwald merged commit 7f8eeba into master Sep 28, 2020

ngreenwald deleted the deepcell-task branch September 28, 2020 18:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deepcell task #199

Deepcell task #199

omerbt commented Aug 25, 2020 •

edited

Loading

ngreenwald left a comment

omerbt commented Aug 25, 2020 •

edited

Loading

ngreenwald left a comment

ngreenwald Aug 25, 2020

omerbt Aug 25, 2020

ngreenwald commented Aug 25, 2020

ngreenwald commented Aug 26, 2020

omerbt commented Aug 26, 2020

ngreenwald commented Aug 26, 2020

omerbt commented Aug 26, 2020

alex-l-kong commented Sep 25, 2020 •

edited

Loading

alex-l-kong commented Sep 25, 2020

ngreenwald commented Sep 25, 2020

ngreenwald left a comment

alex-l-kong left a comment

alex-l-kong Sep 26, 2020 •

edited

Loading

omerbt Sep 27, 2020

alex-l-kong Sep 26, 2020

ngreenwald Sep 26, 2020

alex-l-kong Sep 27, 2020

alex-l-kong Sep 26, 2020 •

edited

Loading

omerbt Sep 27, 2020

ngreenwald commented Sep 27, 2020

alex-l-kong commented Sep 27, 2020

ackagel left a comment

ackagel Sep 28, 2020

omerbt Sep 28, 2020

omerbt commented Sep 28, 2020

alex-l-kong left a comment

alex-l-kong Sep 28, 2020

omerbt Sep 28, 2020

alex-l-kong left a comment

ngreenwald left a comment

ackagel left a comment

		warnings.warn(f'Deep Cell output file was not found for {point}.')


		def run_deepcell_task(input_dir, output_dir, host='https://deepcell.org',

Deepcell task #199

Deepcell task #199

Conversation

omerbt commented Aug 25, 2020 • edited Loading

ngreenwald left a comment

Choose a reason for hiding this comment

omerbt commented Aug 25, 2020 • edited Loading

ngreenwald left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngreenwald commented Aug 25, 2020

ngreenwald commented Aug 26, 2020

omerbt commented Aug 26, 2020

ngreenwald commented Aug 26, 2020

omerbt commented Aug 26, 2020

alex-l-kong commented Sep 25, 2020 • edited Loading

alex-l-kong commented Sep 25, 2020

ngreenwald commented Sep 25, 2020

ngreenwald left a comment

Choose a reason for hiding this comment

alex-l-kong left a comment

Choose a reason for hiding this comment

alex-l-kong Sep 26, 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

alex-l-kong Sep 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngreenwald commented Sep 27, 2020

alex-l-kong commented Sep 27, 2020

ackagel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omerbt commented Sep 28, 2020

alex-l-kong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex-l-kong 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

omerbt commented Aug 25, 2020 •

edited

Loading

omerbt commented Aug 25, 2020 •

edited

Loading

alex-l-kong commented Sep 25, 2020 •

edited

Loading

alex-l-kong Sep 26, 2020 •

edited

Loading

alex-l-kong Sep 26, 2020 •

edited

Loading