-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(hugr-py)!: pretty printing for ops and types #1482
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1482 +/- ##
==========================================
+ Coverage 87.95% 88.00% +0.05%
==========================================
Files 125 125
Lines 21114 21244 +130
Branches 18272 18272
==========================================
+ Hits 18570 18695 +125
- Misses 1760 1765 +5
Partials 784 784
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
hugr-py/tests/conftest.py
Outdated
if os.environ.get("HUGR_RENDER_DOT"): | ||
with NamedTemporaryFile(mode="w", delete=True) as f: | ||
# test rendering succeeds | ||
dot.render(f.name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use dot.pipe("svg")
instead of writing to a file.
It should be much quicker.
https://graphviz.readthedocs.io/en/stable/manual.html#piped-output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
much simpler, thanks (though it is similarly slow)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, the only thing I'm not sure about are generic function types (see comment below)
hugr-py/src/hugr/tys.py
Outdated
def __str__(self) -> str: | ||
# ∀[a]. [list<a>] -> [Bool] | ||
return f"∀{self.params}. {self.body!s})" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would look more like ∀[TypeTypeParam(TypeBound.Any)].[list<Variable(0)>] -> [Bool]
We probably want more concise __str__
implementations for TypeParam
s. Also maybe we should print sth like $0
instead of Variable(0)
(same for VariableArg
)?
hugr-py/src/hugr/tys.py
Outdated
def __str__(self) -> str: | ||
return f"{self.type_def.name}<{', '.join(str(arg) for arg in self.args)}>" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<>
should be left out if self.args
is empty
hugr-py/src/hugr/tys.py
Outdated
def __str__(self) -> str: | ||
return f"{self.id}<{', '.join(str(arg) for arg in self.args)}>" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
BREAKING CHANGE: rename Custom.name to Custom.op_name to allow for new `name` method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the tests!
🤖 I have created a release *beep* *boop* --- ## [0.8.0](hugr-py-v0.7.0...hugr-py-v0.8.0) (2024-08-30) ### ⚠ BREAKING CHANGES * Moved `itobool`/`ifrombool`/`itostring_{u,s}` to the conversions extension. * Binary sums representing fallible values now use tag `1` for the successful variant * **hugr-py:** rename `Custom.name` to `Custom.op_name` and `Func(Defn/Decl).name` to `f_name` to allow for new `name` method * `ListOp::pop` now returns an option. * Moved all builder definitions into the `hugr.build` module. Moved `node_port` and `render` into the `hugr.hugr` module. * Move `Lift`, `MakeTuple`, `UnpackTuple` and `Lift` from core operations to prelude. Rename `ops::leaf` module to `ops::sum`. * **hugr-py:** `hugr.serialization` module and `to_serial` methods are now internal only. * Renamed `_DfBase` to `DfBase` and `_DefinitionBuilder` to `DefinitionBuilder` * `idivmod_checked`, `idivmod`, `idiv`, `idiv_checked`, `imod`, `ishl`, `ishr`, `irotl`, `irotr` operations now only have one width argument for all inputs and outputs rather than two. * HUGRs containing opaque operations that don't point to an extension in the registry will fail to validate. Use `Package` to pack extensions with HUGRs for serialisation. * `CustomOp` removed, `OpType` now contains `ExtensionOp` and `OpaqueOp` directly. `CustomOpError` renamed to`OpaqueOpError`. ### Features * `Option` / `Result` helpers ([#1481](#1481)) ([9698420](9698420)) * Add missing ops ([#1463](#1463)) ([841f450](841f450)) * Add more list operations ([#1474](#1474)) ([037005f](037005f)) * Bring in the pure-python renderer from guppy ([#1462](#1462)) ([001e66a](001e66a)) * disallow opaque ops during validation ([#1431](#1431)) ([fbbb805](fbbb805)) * fill out array ops ([#1491](#1491)) ([26ec57a](26ec57a)) * **hugr-py:** pretty printing for ops and types ([#1482](#1482)) ([aca403a](aca403a)) * **hugr-py:** use serialized extensions in python ([#1459](#1459)) ([a61f4df](a61f4df)), closes [#1450](#1450) * int operations other than widen/narrow have only one width arg ([#1455](#1455)) ([c39ed15](c39ed15)) * Move `Lift`, `MakeTuple`, `UnpackTuple` and `Lift` to prelude ([#1475](#1475)) ([b387505](b387505)) * Move int conversions to `conversions` ext, add to/from usize ([#1490](#1490)) ([88913f2](88913f2)) ### Bug Fixes * **hugr-py:** record node children correctly when deserializing ([#1480](#1480)) ([331125a](331125a)), closes [#1479](#1479) * **py:** Invalid node indexing ([#1457](#1457)) ([d6edcd7](d6edcd7)) * schema for array inner types ([#1494](#1494)) ([d43cbb2](d43cbb2)), closes [#1471](#1471) * Sum value equality. Add unit tests ([#1484](#1484)) ([a7b2718](a7b2718)) ### Code Refactoring * flatten `CustomOp` in to `OpType` ([#1429](#1429)) ([8e8bba5](8e8bba5)) * **hugr-py:** make serialization (module/methods) private ([#1477](#1477)) ([49a5bad](49a5bad)) * Make `_DfBase` and `_DefinitionBuilder` public ([#1461](#1461)) ([ea9cca0](ea9cca0)) * Make Either::Right the "success" case ([#1489](#1489)) ([8caa572](8caa572)) * Organise the hugr-py modules ([#1460](#1460)) ([3ca56f4](3ca56f4)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: Agustín Borgna <121866228+aborgna-q@users.noreply.github.com> Co-authored-by: Agustin Borgna <agustin.borgna@quantinuum.com>
Closes #1468
Closes #1470
Previous testing wasn't covering label formatting because it requires actually rendering the dot in to an image. Have enabled that, but put it behind an env var because it significantly slows down tests.
BREAKING CHANGE: rename
Custom.name
toCustom.op_name
andFunc(Defn/Decl).name
tof_name
to allow for newname
method