Recipe runner CLI entry point #1160

ttung · 2019-04-10T02:38:01Z

Add the recipe runner CLI entry point.
Add a recipe for the ISS pipeline and a test that exercises the recipe.

Test plan: pytest starfish/test/full_pipelines/recipe

Depends on #1192

Fixes #311

codecov-io · 2019-04-10T02:50:56Z

Codecov Report

Merging #1160 into tonytung-recipe will increase coverage by 0.2%.
The diff coverage is 94.73%.

@@                Coverage Diff                 @@
##           tonytung-recipe    #1160     +/-   ##
==================================================
+ Coverage            88.66%   88.86%   +0.2%     
==================================================
  Files                  137      140      +3     
  Lines                 5168     5244     +76     
==================================================
+ Hits                  4582     4660     +78     
+ Misses                 586      584      -2

Impacted Files	Coverage Δ
starfish/__init__.py	`90.9% <100%> (+0.43%)`	⬆️
starfish/test/full_pipelines/recipe/test_iss.py	`100% <100%> (ø)`
starfish/recipe/cli.py	`100% <100%> (ø)`
...sh/test/full_pipelines/recipe/_base_recipe_test.py	`86.2% <86.2%> (ø)`
starfish/recipe/runnable.py	`96.66% <0%> (+1.11%)`	⬆️
starfish/recipe/filesystem.py	`97.95% <0%> (+2.04%)`	⬆️
starfish/recipe/recipe.py	`98.95% <0%> (+3.12%)`	⬆️
... and 1 more

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 a780b19...9b8329b. Read the comment docs.

ambrosejcarr

Awesome work. Looking for docs. 👀

ambrosejcarr · 2019-04-14T21:41:27Z

starfish/recipe/cli.py

+    "--output", type=str, multiple=True, metavar="OUTPUT_FILE_PATH",
+    help="output file paths to write recipe outputs to")
+@click.pass_context
+def run_recipe(ctx, recipe, input, output):


This needs usage examples. It's also not clear what any of the parameters must contain for the run to be valid. That should probably be documented here.

It is recipe dependent. Please let me know if #1245 is adequate.

ambrosejcarr · 2019-04-14T21:50:52Z

starfish/recipe/cli.py

+    """Runs a recipe with a given set of inputs and outputs."""
+    config = StarfishConfig()
+
+    backend, relativeurl, _ = resolve_path_or_url(


General style question: We have a bunch of custom classes, and in this case I needed to follow 3 wrapper functions in slicedimage.io to find out that this backend is a subclass of slicedimage.backends.Backend.

Since we have type hints, I would find it helpful to use them in the cases of our custom classes, so that developers have a better sense of what the code is doing.

In this case:

backend: slicedimage.io.Backend relativeurl: str backend, relativeurl, _ = resolve_path_or_url( recipe, backend_config=config.slicedimage)

Sure, I can do this.

Correction: Backend was never exposed.

I will correct this.

ambrosejcarr · 2019-04-14T21:53:26Z

starfish/recipe/cli.py

+
+
+@starfish.command("recipe")
+@click.option("--recipe", required=True, type=str, metavar="RECIPE_PATH_OR_URL")


I wonder why we have a public run method now that we can run recipes and you've shown this to be faster.

For API users.

ambrosejcarr · 2019-04-14T21:54:09Z

starfish/test/full_pipelines/recipe/iss_recipe.txt

@@ -0,0 +1,54 @@
+primary_image = file_inputs[0]


This is great. We need to doc how this works and what your mini-language can do.

shanaxel42 · 2019-04-15T20:06:33Z

starfish/test/full_pipelines/recipe/iss_recipe.txt

+    "decode", "PerRoundMaxChannelDecoder",
+    target_assignment, codebook)
+
+file_outputs[0] = decoded


does this mean you only output once at the end of the pipeline?
That would definitely explain why this process is way faster the exporting and importing after each step

Yes, that is true. The speed improvements (IMO) is mostly from not starting up the starfish process repeatedly.

1. Add the recipe runner CLI entry point. 2. Add a recipe for the ISS pipeline and a test that exercises the recipe. Test plan: `pytest starfish/test/full_pipelines/recipe` Depends on #1117 Fixes #311

ttung · 2019-04-23T04:30:05Z

I'm going to land this, free free to leave any post-land comments, @ambrosejcarr / @kne42

ttung requested review from ambrosejcarr, shanaxel42 and kne42 April 10, 2019 02:38

ttung force-pushed the tonytung-recipe-cli branch from 0a09822 to b90d84d Compare April 10, 2019 18:54

ttung force-pushed the tonytung-recipe branch from 89d75bb to 8860ad4 Compare April 10, 2019 19:53

ttung force-pushed the tonytung-recipe-cli branch 3 times, most recently from a52a19e to 72ac99c Compare April 11, 2019 18:28

ttung force-pushed the tonytung-recipe branch from 4aa1595 to 9989f28 Compare April 12, 2019 03:37

ttung force-pushed the tonytung-recipe-cli branch from 72ac99c to 3ed2d41 Compare April 12, 2019 03:37

ambrosejcarr requested changes Apr 14, 2019

View reviewed changes

ttung force-pushed the tonytung-recipe branch from 9989f28 to ccffa54 Compare April 15, 2019 04:08

ttung force-pushed the tonytung-recipe-cli branch from 3ed2d41 to a1df8d5 Compare April 15, 2019 04:08

shanaxel42 approved these changes Apr 15, 2019

View reviewed changes

ttung force-pushed the tonytung-recipe-cli branch from a1df8d5 to b626228 Compare April 16, 2019 16:00

ttung force-pushed the tonytung-recipe branch from 7ce5932 to f09f72c Compare April 16, 2019 16:13

ttung force-pushed the tonytung-recipe-cli branch from b626228 to d12119c Compare April 16, 2019 16:13

ttung force-pushed the tonytung-recipe branch from f09f72c to 77c21ec Compare April 17, 2019 02:39

ttung force-pushed the tonytung-recipe-cli branch from d12119c to 741cfa1 Compare April 17, 2019 02:39

ttung force-pushed the tonytung-recipe branch from 77c21ec to 501f326 Compare April 17, 2019 03:38

ttung force-pushed the tonytung-recipe-cli branch from 741cfa1 to 3a22448 Compare April 17, 2019 03:38

ttung force-pushed the tonytung-recipe branch from 501f326 to e5c2ef4 Compare April 17, 2019 04:52

ttung force-pushed the tonytung-recipe-cli branch 4 times, most recently from 95c4e7a to 487113c Compare April 17, 2019 17:29

ttung force-pushed the tonytung-recipe branch from 0e47255 to c97d827 Compare April 18, 2019 00:11

ttung force-pushed the tonytung-recipe-cli branch 2 times, most recently from a2a7266 to ae0df91 Compare April 18, 2019 03:33

ttung force-pushed the tonytung-recipe-cli branch from 483d545 to 1ba8b25 Compare April 20, 2019 06:23

ttung force-pushed the tonytung-recipe branch from cb07126 to 920b571 Compare April 21, 2019 17:11

ttung force-pushed the tonytung-recipe-cli branch 2 times, most recently from 7724f21 to 715e3d8 Compare April 22, 2019 00:07

ttung force-pushed the tonytung-recipe branch from 05caa43 to c7ef79c Compare April 22, 2019 00:10

ttung force-pushed the tonytung-recipe-cli branch 3 times, most recently from 3d55d00 to 62295fc Compare April 22, 2019 06:04

ttung force-pushed the tonytung-recipe branch from 5e30e54 to cb1d95a Compare April 22, 2019 18:44

ttung force-pushed the tonytung-recipe-cli branch from 62295fc to 538cb0d Compare April 22, 2019 18:44

ttung force-pushed the tonytung-recipe branch from cb1d95a to e8bb437 Compare April 22, 2019 19:25

ttung force-pushed the tonytung-recipe-cli branch 3 times, most recently from 9957b8d to ab77e27 Compare April 22, 2019 20:21

ttung force-pushed the tonytung-recipe branch from 06d6544 to ec53c50 Compare April 22, 2019 20:25

ttung force-pushed the tonytung-recipe-cli branch from ab77e27 to 50b80ed Compare April 22, 2019 20:25

ttung force-pushed the tonytung-recipe branch from ec53c50 to 61dd567 Compare April 22, 2019 21:50

ttung force-pushed the tonytung-recipe-cli branch 2 times, most recently from 28fcd4f to 612307c Compare April 22, 2019 22:35

ttung mentioned this pull request Apr 22, 2019

Documentation for running a recipe. #1245

Merged

ttung force-pushed the tonytung-recipe-cli branch from 612307c to 742bb54 Compare April 22, 2019 22:40

ttung requested a review from ambrosejcarr April 22, 2019 22:40

ttung force-pushed the tonytung-recipe branch from 61dd567 to a780b19 Compare April 23, 2019 00:38

ttung force-pushed the tonytung-recipe-cli branch from 742bb54 to 9b8329b Compare April 23, 2019 00:38

ttung changed the base branch from tonytung-recipe to master April 23, 2019 04:24

Recipe runner CLI entry point

d74a613

1. Add the recipe runner CLI entry point. 2. Add a recipe for the ISS pipeline and a test that exercises the recipe. Test plan: `pytest starfish/test/full_pipelines/recipe` Depends on #1117 Fixes #311

ttung force-pushed the tonytung-recipe-cli branch from 9b8329b to d74a613 Compare April 23, 2019 04:26

ttung merged commit efa9d78 into master Apr 23, 2019

ttung deleted the tonytung-recipe-cli branch April 23, 2019 05:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Recipe runner CLI entry point #1160

Recipe runner CLI entry point #1160

ttung commented Apr 10, 2019 •

edited

Loading

codecov-io commented Apr 10, 2019 •

edited

Loading

ambrosejcarr left a comment

ambrosejcarr Apr 14, 2019

ttung Apr 22, 2019

ambrosejcarr Apr 14, 2019

ttung Apr 22, 2019

ttung Apr 22, 2019

ttung Apr 22, 2019

ambrosejcarr Apr 14, 2019

ttung Apr 22, 2019

ambrosejcarr Apr 14, 2019

ttung Apr 22, 2019

shanaxel42 Apr 15, 2019

ttung Apr 22, 2019

ttung commented Apr 23, 2019



		@starfish.command("recipe")
		@click.option("--recipe", required=True, type=str, metavar="RECIPE_PATH_OR_URL")

Recipe runner CLI entry point #1160

Recipe runner CLI entry point #1160

Conversation

ttung commented Apr 10, 2019 • edited Loading

codecov-io commented Apr 10, 2019 • edited Loading

Codecov Report

ambrosejcarr 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ttung commented Apr 23, 2019

ttung commented Apr 10, 2019 •

edited

Loading

codecov-io commented Apr 10, 2019 •

edited

Loading