Recipe and recipe execution #1117
Conversation
|
This lacks tests, documentation, and probably has bugs. But I wanted to get this out in front of you guys sooner rather than later. |
Codecov Report
@@ Coverage Diff @@
## tonytung-runnable #1117 +/- ##
=====================================================
+ Coverage 89.07% 89.31% +0.24%
=====================================================
Files 131 132 +1
Lines 4980 5065 +85
=====================================================
+ Hits 4436 4524 +88
+ Misses 544 541 -3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I read the tests, everything looks good from a user perspective. I didn't read the code in detail (can I leave @shanaxel42 to do that part of the review?)
You're going to add some documentation about how a user would use this, right? :-)
|
|
||
| result_stack = ImageStack.from_path_or_url(output_path) | ||
| assert np.allclose( | ||
| BASE_EXPECTED * .5, |
There was a problem hiding this comment.
Testing suggestion:
Change the multiplicands to 0.5, 2.0, 4.0, and the expected to BASE_EXPECTED * 4; as the test stands, it would pass if only the first compute call executed for some reason.
There was a problem hiding this comment.
I did something similar to what you suggested.
| self.runnable_dependents: Mapping[Runnable, AbstractSet[Runnable]] = runnable_dependents | ||
|
|
||
| # completed results | ||
| self._completed_runnables: Set[Runnable] = set() |
There was a problem hiding this comment.
Does this imply that you could never have duplicate runnables?
There was a problem hiding this comment.
Yes, even runnables with the same parameters are treated as distinct entities.
starfish/recipe/recipe.py
Outdated
| while True: | ||
| candidate_runnable = next(self._runnable_sequence) | ||
|
|
||
| if candidate_runnable in self.needed_runnables: |
There was a problem hiding this comment.
what exactly does self.needed_runnables mean in this context?
There was a problem hiding this comment.
As discussed offline, this is undesirable, and as such, removed.
starfish/recipe/recipe.py
Outdated
| if dependency not in runnable_dependents: | ||
| runnable_dependents[dependency] = set() | ||
| runnable_dependents[dependency].add(runnable) | ||
| Execution.build_graph(dependency, needed_runnables, runnable_dependents) |
| "compute": ordered_sequence, | ||
| "file_outputs": outputs, | ||
| } | ||
| ast = compile(recipe_str, "<string>", "exec") |
There was a problem hiding this comment.
what does ast stand for?
There was a problem hiding this comment.
abstract syntax tree.
starfish/recipe/recipe.py
Outdated
| ast = compile(recipe_str, "<string>", "exec") | ||
| exec(ast, recipe_scope) | ||
|
|
||
| assert len(outputs) == len(output_paths), \ |
ttung
force-pushed
the
tonytung-recipe
branch
3 times, most recently
from
d680299 to
9458cd6
Compare
ttung
force-pushed
the
tonytung-recipe
branch
4 times, most recently
from
a56438c to
f220346
Compare
ttung
force-pushed
the
tonytung-recipe
branch
2 times, most recently
from
e4db852 to
caeb9b7
Compare
ttung
force-pushed
the
tonytung-recipe
branch
2 times, most recently
from
dc3e260 to
c1fb51b
Compare
ttung
force-pushed
the
tonytung-recipe
branch
4 times, most recently
from
c3f09a5 to
dcd0212
Compare
Builds on top of #1097 to construct and run recipes with one or more runnables.
A recipe wires together one or more runnables. Inputs to recipes are available to the recipe itself as the magic variable name "file_inputs". A runnable is invoked by the magic function "compute". Anything written to the magic variable "file_outputs" is matched and written out to a set of output paths.
Fixes #311