Conversation
And a toy WfMS Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #185 +/- ##
==========================================
+ Coverage 97.96% 99.23% +1.26%
==========================================
Files 34 32 -2
Lines 2259 2598 +339
==========================================
+ Hits 2213 2578 +365
+ Misses 46 20 -26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
So child `from_recipe` definitions don't violate lsp Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
To avoid mypy complaining about the implicit type change Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Again, to satisfy mypy re the otherwise implicit type change Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Input being iterated on had better be iterable, or better a Collection! We _can't_ enforce this at the recipe level, since it's runtime behaviour. Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
I didn't mind the edges being the same object such that edge topology was computationally forced to stay linked between live and recipe forms; but this is _not_ how it works for the nodes, which have a different data format and thus _cannot_ simply be the same object. So, for consistency between node and edge (and IO) behaviour, I think it's better to shallow copy the edges. Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
And add tests Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
For new features Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
expectations are met Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
And make the error message more explicit about the fact this is a fall-back Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
and running, including custom output names Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
The version file is dynamic and not part of the code base. I'm having trouble getting codecov to ignore the api submodule, so there let's try a wildcard for path matching. Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Honestly, I'm not sure the `nodes` content should be duplicated in the `schema` either, but at least nodes are a bit of a special(y important) case. For now just don't make things worse, and stick all the schema-like stuff in schemas and leave the user-facing function in its own api module. Overall the api probably needs to re-pathing anyway. Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Instead of coveragerc, which (probably?) had its omits overwritten by the CLI Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
|
@samwaseda, one of the key differences to the It can sometimes be a bit more of a pain to write, but I find it reads much easier. Delaying the arg-vs-kwarg until we finally call the underlying atomic python function saves some maintenance headache too, since then it only becomes relevant at exactly one spot. |
There was a problem hiding this comment.
Pull request overview
This PR introduces “live” (runtime) node objects and a minimal toy Workflow Management System (WfMS) that can execute flowrep recipes into populated live graphs, along with a comprehensive unit test suite to validate execution behavior across atomic/workflow/flow-control recipes.
Changes:
- Added
flowrep.models.livelive-node data structures (ports, atomic/composite nodes, recipe→live conversion). - Added
flowrep.models.wfmsminimal executor (run_recipe) supporting atomic, workflow, for/while/if/try nodes. - Added extensive unit tests and expanded the static test library with additional atomic functions.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
flowrep/models/live.py |
New live node/port structures and function signature/type parsing for live ports. |
flowrep/models/wfms.py |
New minimal WfMS to execute recipes and populate live graphs and port values. |
flowrep/models/api/live.py |
API re-export for live recipe→live conversion. |
flowrep/models/api/wfms.py |
API re-export for run_recipe. |
flowrep/models/api/schemas.py |
Exposes live classes/types through the API schemas module. |
flowrep/models/parsers/atomic_parser.py |
Tweaks the output-label mismatch error message. |
flowrep/models/nodes/for_model.py |
Documentation update clarifying runtime expectations for iterated inputs. |
tests/unit/models/test_live_wfms.py |
New end-to-end unit tests covering live models and WfMS execution. |
tests/flowrep_static/library.py |
Adds more atomic functions used by the new WfMS/live tests. |
.github/workflows/push-pull.yml |
Adjusts CI coverage omission patterns for API modules. |
.coveragerc |
Simplifies coverage config (removes omit list). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
for input port data Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
By allowing multiple unhinted returns; explicitly checking for tuple returns on hinted splitting; and maintaining our length check. Error messages now include suggestions. Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
|
Since the live data structures have room for annotations, I this PR gets far enough to make me comfortable closing #17. The rest is then application in other repos, and edits here to incorporate feedback from those experiences. |
Signed-off-by: liamhuber <liamhuber@greyhavensolutions.com>
And a toy WfMS. Follows the concept of the mutable instance views being separate, not-easily-serializable, and living alongside and coupled to their underlying recipe. All the retrospective views with internal data are structured to be plain DAGs.
I'm not even sure these should live in this repo, but I want a place to play around and have conversations against real code examples.
EDIT:
This turned out better than I'd hoped.
It introduces a new
LiveNodedata class, to be a basis for instance-views of recipes. This holds an associatedrecipefield, andinput_portsandoutput_ports. The ports should have all the same keys as therecipe.inputsandrecipe.outputs, but the ports make space for values and annotations (and defaults, in the case of input ports).This basis is then extended for
Atomic(LiveNode), which is associated withAtomicNoderecipes and additionally holds a livefunctionfield (from instantiating thePythonReferencein the recipe), andWorkflow(Composite(LiveNode))andFlowControl(Composite(LiveNode))subclasses, which all havenodes: MutableMapping[base_models.Label, LiveNode], and dictionaries forinput_edges,edges, andoutput_edges. The difference between the last two is that theWorkflowlive node knows all its child instances and edges from the get-go, so they can be populated at creation. The flow controls know only prospective nodes and edges from their recipes, so these are instantiated as empty containers and it's up to the WfMS to fill them at runtime.The primary API is a
recipe2livefunction, but you can also.from_recipetheAtomic,Workflow, andFlowControlclasses directly if you already know what sort of recipe you've got.Then there is a WfMS module with a toy implementation. It has an API
run_recipewhich converts a recipe object into an output-data-populated live object.@samwaseda, FYI this is working basically as I envisioned and I'm pretty happy with it. What I'll do now is try and build a converter from a
Workflowlive node to the legacy nested dictionary. This might turn up some pieces that are missing here. Once that conversion is working, it works then the conversion and.runmethod binding can both be done over insemantikon, and then@semantikon.workflow.workflowcan directly extend@flowrep.models.api.workflow.EDIT:
And a super simple example