feat: Make ComposablePass.__call__ return a Hugr#2697
Merged
Conversation
aborgna-q
reviewed
Nov 18, 2025
aborgna-q
reviewed
Nov 18, 2025
aborgna-q
reviewed
Nov 18, 2025
aborgna-q
reviewed
Nov 18, 2025
aborgna-q
reviewed
Nov 18, 2025
aborgna-q
reviewed
Nov 18, 2025
| passes: list[ComposablePass] | ||
|
|
||
| def __call__(self, hugr: Hugr): | ||
| def __call__(self, hugr: Hugr, inplace: bool = True) -> Hugr: |
Collaborator
There was a problem hiding this comment.
Make sure to overrite _apply and _apply_inline, in case some implementor calls them directly
CalMacCQ
changed the title
ComposablePass return a HugrComposablePass.__call__ return a Hugr
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2697 +/- ##
==========================================
- Coverage 83.54% 83.48% -0.06%
==========================================
Files 266 266
Lines 51714 51650 -64
Branches 47180 47086 -94
==========================================
- Hits 43202 43121 -81
- Misses 6134 6153 +19
+ Partials 2378 2376 -2
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:
|
acl-cqc
reviewed
Nov 19, 2025
Contributor
acl-cqc
left a comment
acl-cqc
left a comment
There was a problem hiding this comment.
Thanks, Callum - think the interface is ok, I don't have a great solution to the override-at-least-one problem, but a couple of things....
| """Call all of the passes in sequence.""" | ||
| def _apply(self, hugr: Hugr) -> Hugr: | ||
| for comp_pass in self.passes: | ||
| res = comp_pass(hugr, inplace=False) |
Contributor
There was a problem hiding this comment.
This returns the result of the last pass only! You should probably initialize via res = hugr outside the loop and then
Suggested change
| res = comp_pass(hugr, inplace=False) | |
| res = comp_pass(res, inplace=False) |
I'm surprised mypy doesn't complain about this, as if self.passes is empty then return res is returning an uninitialized variable
| # At least one of the following _apply methods must be ovewritten | ||
| def _apply(self, hugr: Hugr) -> Hugr: | ||
| hugr = deepcopy(hugr) | ||
| self._apply_inplace(hugr) |
Contributor
There was a problem hiding this comment.
If we were really concerned....you could have an _in_progress:bool on the instance of ComposablePass (not sure but this might mean setattr??), and then check that you didn't get back here with that set (and error if you do)....
I don't see a great solution tho, no
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 24, 2025
Includes an idea for simplifying the protocol's `_apply`/`_apply_inline` from #2697 by providing a helper function instead (859c811). --------- Co-authored-by: Callum Macpherson <[email protected]> Co-authored-by: Callum Macpherson <[email protected]> Co-authored-by: Alan Lawrence <[email protected]>
aborgna-q
added a commit
that referenced
this pull request
Nov 24, 2025
Implementing the revisions to `ComposablePass` and `ComposedPass` as discussed with Agustin. I still find the `_apply` and `_apply_inplace` rather unintuitive (although now I realise that it will work). Looking at the default implementations in the `ComposablePass` protocol these look circular but as you're always overrididing at least one of the `_apply` or `_apply_inplace` methods whenever you implement the protocol. Would be interested to know if there are suggestions to improve this. --------- Co-authored-by: Agustín Borgna <[email protected]> Co-authored-by: Agustín Borgna <[email protected]>
aborgna-q
added a commit
that referenced
this pull request
Nov 24, 2025
Includes an idea for simplifying the protocol's `_apply`/`_apply_inline` from #2697 by providing a helper function instead (859c811). --------- Co-authored-by: Callum Macpherson <[email protected]> Co-authored-by: Callum Macpherson <[email protected]> Co-authored-by: Alan Lawrence <[email protected]>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 5, 2026
🤖 I have created a release *beep* *boop* --- ## [0.15.0](hugr-py-v0.14.2...hugr-py-v0.15.0) (2026-01-02) ### ⚠ BREAKING CHANGES * ValueArray is gone, as is LinearizeArrayPass * **hugr-py:** Removes `extensions` field of `val.Extension`. ### Features * `collated_digitstring_counts` to extend bitstring collation to digits ([#2788](#2788)) ([191c473](191c473)) * **hugr-py:** Remove `extensions` field of `val.Extension`. ([#2686](#2686)) ([911c089](911c089)) * Make `ComposablePass.__call__` return a Hugr ([#2697](#2697)) ([dbf8c8e](dbf8c8e)) * Result type for ComposablePasses ([#2703](#2703)) ([b8df28e](b8df28e)) ### Bug Fixes * **hugr-py:** solved graph rendering with `Const` nodes after applying`NormalizeGuppy` ([#2744](#2744)) ([d996690](d996690)) * set hugr field of `PassResult` correctly ([#2715](#2715)) ([d860722](d860722)) ### Code Refactoring * Delete ValueArray ([#2760](#2760)) ([b3cdc4e](b3cdc4e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Implementing the revisions to
ComposablePassandComposedPassas discussed with Agustin.I still find the
_applyand_apply_inplacerather unintuitive (although now I realise that it will work). Looking at the default implementations in theComposablePassprotocol these look circular but as you're always overrididing at least one of the_applyor_apply_inplacemethods whenever you implement the protocol.Would be interested to know if there are suggestions to improve this.