Skip to content

refactor(hugr-model/binary/read.rs): common up 4-6* code for reading symbols #2343

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

Closed
wants to merge 3 commits into from

Conversation

acl-cqc
Copy link
Contributor

@acl-cqc acl-cqc commented Jun 16, 2025

No description provided.

Copy link

codecov bot commented Jun 16, 2025

Codecov Report

Attention: Patch coverage is 66.66667% with 8 lines in your changes missing coverage. Please review.

Project coverage is 82.12%. Comparing base (d79b738) to head (3cdbb5b).
Report is 50 commits behind head on main.

Files with missing lines Patch % Lines
hugr-model/src/v0/binary/read.rs 66.66% 0 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2343      +/-   ##
==========================================
- Coverage   82.13%   82.12%   -0.01%     
==========================================
  Files         241      241              
  Lines       44018    43976      -42     
  Branches    39857    39810      -47     
==========================================
- Hits        36154    36117      -37     
- Misses       5870     5874       +4     
+ Partials     1994     1985       -9     
Flag Coverage Δ
python 85.40% <ø> (-0.08%) ⬇️
rust 81.78% <66.66%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@acl-cqc acl-cqc changed the title refactor(hugr-model/binary/read.rs) common up 3-5* code for reading symbols refactor(hugr-model/binary/read.rs): common up 3-5* code for reading symbols Jun 16, 2025
@acl-cqc acl-cqc requested a review from zrho June 16, 2025 17:28
@acl-cqc acl-cqc marked this pull request as ready for review June 16, 2025 17:28
@acl-cqc acl-cqc requested a review from a team as a code owner June 16, 2025 17:28
@acl-cqc acl-cqc changed the title refactor(hugr-model/binary/read.rs): common up 3-5* code for reading symbols refactor(hugr-model/binary/read.rs): common up 4-6* code for reading symbols Jun 16, 2025
acl-cqc added a commit that referenced this pull request Jun 17, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jul 9, 2025
Note: modified to come after #2412 as I want those Schema checks in
place to check this works!
closes #2354, #1752.
Includes cherry-pick of #2343.

* Add `enum Visibility`. In fact *2 (hugr-model version is not
serde-able so cannot be used in hugr-core).
* `FuncDefn` and `FuncDecl` gain this. (A good use-case for a private
FuncDecl is building cyclic functions via builder `define_declaration`.)
* I've not added to `AliasDecl` or `AliasDefn` - I could, it would be
consistent, but we hardly use them....
* `validate` checks that the *public* children of a Module have unique
names (except multiple FuncDecls may alias).

* `FuncDefn::new` defaults to private (also builder `define_function` /
`FunctionBuilder::new`, and json-deserialize in both python and rust).
There is a `_vis` taking an explicit Visibility parameter.
* `FuncDecl::new` defaults to public (also builder `declare_function`,
and json-deserialize*2). Again there is `_vis`.
* hugr-model import/export + roundtrip + AST printing via new keyword
`pub`. Note since `hugr-core` does not have visibility on aliases or
extension operations I've defaulted these to private to avoid any change
to textual output.
* add field to hugr-py, using "string"-enum (`Literal["Public",
"Private"]`)
* I've attempted to add hugr-model hugr-py bindings, but there are no
tests of these yet.

Note all changes to hugr-passes delayed until a follow-up PR.

BREAKING CHANGE: hugr-model: Symbol has an extra field
@acl-cqc
Copy link
Contributor Author

acl-cqc commented Jul 9, 2025

Went in as part of #2143

@acl-cqc acl-cqc closed this Jul 9, 2025
lmondada pushed a commit that referenced this pull request Jul 9, 2025
Note: modified to come after #2412 as I want those Schema checks in
place to check this works!
closes #2354, #1752.
Includes cherry-pick of #2343.

* Add `enum Visibility`. In fact *2 (hugr-model version is not
serde-able so cannot be used in hugr-core).
* `FuncDefn` and `FuncDecl` gain this. (A good use-case for a private
FuncDecl is building cyclic functions via builder `define_declaration`.)
* I've not added to `AliasDecl` or `AliasDefn` - I could, it would be
consistent, but we hardly use them....
* `validate` checks that the *public* children of a Module have unique
names (except multiple FuncDecls may alias).

* `FuncDefn::new` defaults to private (also builder `define_function` /
`FunctionBuilder::new`, and json-deserialize in both python and rust).
There is a `_vis` taking an explicit Visibility parameter.
* `FuncDecl::new` defaults to public (also builder `declare_function`,
and json-deserialize*2). Again there is `_vis`.
* hugr-model import/export + roundtrip + AST printing via new keyword
`pub`. Note since `hugr-core` does not have visibility on aliases or
extension operations I've defaulted these to private to avoid any change
to textual output.
* add field to hugr-py, using "string"-enum (`Literal["Public",
"Private"]`)
* I've attempted to add hugr-model hugr-py bindings, but there are no
tests of these yet.

Note all changes to hugr-passes delayed until a follow-up PR.

BREAKING CHANGE: hugr-model: Symbol has an extra field
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant