EDIT: Executable Composites #23
Merged
Conversation
This file contains hidden or 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
Comes with a bit of a change in the API, the `run()` call no longer overrides being in a failed state -- you need to reset the failed state first.
|
|
Pull Request Test Coverage Report for Build 6601723944
💛 - Coveralls |
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
To avoid a difference in meaning to when a node pulls and actually causes upstream executions
And have composite nodes pass just their children (and identify starting nodes) in anticipation of sending composites to an executor
…ou currently have
Instead of "update" and "fetch" separately
Better input interface
As long as that node has no parent or connections, and can successfully copy the connections of the node it is replacing.
We want to avoid ever having a `concurrent.futures.Executor` as an attribute, because it holds a truly unpickleable thread lock object. Instead, we now pass in information _about_ an executor and instantiate one on the fly if we need it. In the medium term we'll want a more sophisticated interface, i.e. information about how to connect to some existing executor object, or more detailed information on what type of executor to instantiate. But for the sake of getting workflows to work on executors, this trivially bool is enough.
So that macro and workflow can lean on the same code, and macro can just extend things a little bit to reflect the fact that it needs to rebuild its IO on unpacking children (workflow _always_ rebuilds its IO, so it's a non-issue)
I had a typo in what I wanted to test for, but it turns out that the test is ill-conceived anyhow, because the futures object is returning new instances each time I ask for the `.result()`, so the `.result()` that the macro got internally will differ from the one in the test
These are a little simpler tests, as the workflow can't have siblings it's connected to
I don't know how to update self attributes right now
…ecutors_for_composite
So that the workflows and macros play well with executors
Executors for composite
These can for sure be dropped when support for <=3.10 is dropped, maybe before.
Works for 3.11 at least
No io by reference for macros
Introduce coupling between input channels
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.
EDIT: Stacked in PRs allow executing Composite nodes in a parallel process; in particular, Macros are now walled-gardens that have their own IO channels instead of the old passing-by-reference that allowed data IO connections to pass through the boundaries of the macro. This didn't get all the way to implementing a
pull()method for Function nodes, but most of the pieces are in-place and it's a sufficiently logical spot to merge things.