Restructure the relationship between PipelineComponent and AlgorithmBase #1095
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This also partially addresses #921 |
ttung
force-pushed
the
tonytung-restructure-pipelines
branch
from
70e9a14 to
d75719f
Compare
Codecov Report
@@ Coverage Diff @@
## master #1095 +/- ##
==========================================
+ Coverage 88.87% 88.89% +0.02%
==========================================
Files 168 168
Lines 6535 6540 +5
==========================================
+ Hits 5808 5814 +6
+ Misses 727 726 -1
Continue to review full report at Codecov.
|
Closed
shanaxel42
approved these changes
Mar 20, 2019
starfish/pipeline/algorithmbase.py
Outdated
|
|
||
|
|
||
| class AlgorithmBaseType(type): | ||
| DISALLOWED_BASES: Set[Type] = set() |
There was a problem hiding this comment.
what's an example of a disallowed base?
There was a problem hiding this comment.
Let me improve the documentation. Basically, we want the subclasses of FilterAlgorithmBase, RegistrationAlgorithmBase, etc., and not the subclasses of AlgorithmBase.
There was a problem hiding this comment.
I went with a different approach that removed the need for this.
ttung
force-pushed
the
tonytung-restructure-pipelines
branch
3 times, most recently
from
112224b to
f9ec78f
Compare
ttung
force-pushed
the
tonytung-restructure-pipelines
branch
from
f9ec78f to
581974c
Compare
Rather than each specific PipelineComponent have a link to the AlgorithmBase that describes the contract that the algorithm must implement, each AlgorithmBase implementation points to the PipelineComponent that it serves. The current arrangement forces all the algorithms to be defined and evaluated in the interpreter before the pipeline component is defined and evaluated. If we want to create an algorithm that's only used in tests, this is not possible. The new arrangement ensures that any implementation of a specific AlgorithmBase is registered with its corresponding PipelineComponent. This adds explicit abstract classes to the AlgorithmBase class hierarchy to make it easier to detect what's an actual algorithm implementation. Depends on #1094 Test plan: `make -j fast`
ttung
force-pushed
the
tonytung-restructure-pipelines
branch
from
581974c to
cbc8192
Compare
ttung
added a commit
that referenced
this pull request
Mar 22, 2019
…ase (#1095) Rather than each specific PipelineComponent have a link to the AlgorithmBase that describes the contract that the algorithm must implement, each AlgorithmBase implementation points to the PipelineComponent that it serves. The current arrangement forces all the algorithms to be defined and evaluated in the interpreter before the pipeline component is defined and evaluated. If we want to create an algorithm that's only used in tests, this is not possible. The new arrangement ensures that any implementation of a specific AlgorithmBase is registered with its corresponding PipelineComponent. This adds explicit abstract classes to the AlgorithmBase class hierarchy to make it easier to detect what's an actual algorithm implementation. Depends on #1094 Test plan: `make -j fast`
ttung
pushed a commit
to ttung/starfish
that referenced
this pull request
Apr 22, 2019
Runnable is the base unit of a recipe. Each runnable constitutes a PipelineComponent with a specific algorithm. Constructor arguments that are FileProviders are loaded into memory and passed in. Inputs to the `run()` method can also include FileProviders. In all cases, FileProviders are associated with a file path or url. The type is inferred from the typing parameters of the method (currently supported: ImageStack, IntensityTable, ExpressionMatrix, and Codebook). Runnables will be wired together to constitute a pipeline recipe. Test plan: Added tests to verify a simple Runnable, chained Runnables, and Runnables that has constructor arguments that are FileProviders. Depends on spacetx#1095
ttung
added a commit
that referenced
this pull request
Apr 22, 2019
Runnable is the base unit of a recipe. Each runnable constitutes a PipelineComponent with a specific algorithm. Constructor arguments that are FileProviders are loaded into memory and passed in. Inputs to the `run()` method can also include FileProviders. In all cases, FileProviders are associated with a file path or url. The type is inferred from the typing parameters of the method (currently supported: ImageStack, IntensityTable, ExpressionMatrix, and Codebook). Runnables will be wired together to constitute a pipeline recipe. Test plan: Added tests to verify a simple Runnable, chained Runnables, and Runnables that has constructor arguments that are FileProviders. Depends on #1095
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Restructure the relationship between PipelineComponent and AlgorithmBase
Rather than each specific PipelineComponent have a link to the AlgorithmBase that describes the contract that the algorithm must implement, each AlgorithmBase implementation points to the PipelineComponent that it serves.
The current arrangement forces all the algorithms to be defined and evaluated in the interpreter before the pipeline component is defined and evaluated. If we want to create an algorithm that's only used in tests, this is not possible.
The new arrangement ensures that any implementation of a specific AlgorithmBase is registered with its corresponding PipelineComponent.
This adds explicit abstract classes to the AlgorithmBase class hierarchy to make it easier to detect what's an actual algorithm implementation.
Depends on #1094
Test plan:
make -j fast