feat: Result type for ComposablePasses #2703
Conversation
aborgna-q
force-pushed
the
ab/composed-pass-result
branch
from
68df8b0 to
567c58d
Compare
aborgna-q
force-pushed
the
ab/composed-pass-result
branch
from
567c58d to
d79a031
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## cm/overwrite_hugr_method #2703 +/- ##
============================================================
+ Coverage 83.50% 83.55% +0.04%
============================================================
Files 266 266
Lines 51733 51754 +21
Branches 47176 47176
============================================================
+ Hits 43201 43242 +41
+ Misses 6152 6132 -20
Partials 2380 2380
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CalMacCQ
left a comment
CalMacCQ
left a comment
There was a problem hiding this comment.
It looks nice, thanks. I find this more intuntive than having _apply and _apply_inplace methods (at least one of which must be overriden when the protocol is implemented.
One question for clarification and also one nit.
| class ComposablePass(Protocol): | ||
| """A Protocol which represents a composable Hugr transformation.""" | ||
|
|
||
| def __call__(self, hugr: Hugr, *, inplace: bool = True) -> Hugr: |
There was a problem hiding this comment.
Now that we have the impl_pass_run function as a helper for implementing ComposablePass.run where is the __call__ method actually used in the pass implementation?
| comp_pass(hugr, inplace=True) | ||
| def __init__(self, *passes: ComposablePass) -> None: | ||
| self.passes = [] | ||
| for pass_ in passes: |
There was a problem hiding this comment.
Nit: is pass_ here intended to be a private variable? If so the convention is usualy to use _ as a prefix rather than a suffix (_pass rather than pass_)
Includes an idea for simplifying the protocol's
_apply/_apply_inlinefrom #2697 by providing a helper function instead (859c811).