Skip to content
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: Ensure packages always have modules at the root #1589

Merged
merged 8 commits into from
Oct 21, 2024

Conversation

aborgna-q
Copy link
Collaborator

Followup to #1587.

Ensures that rust Packages only contain module-rooted hugrs. Returns errors at construction times if the condition is not met.
This required fixing hugr-cli, as it should be able to load both packages and arbitrary hugrs.
Added Package::from_hugr{,s} methods that try to wrap the hugrs if possible. We'll need these for tket2.

Packages are not on the spec yet, their description should include this restriction. See #1388.

This PR does only modify the (unpublished) API introduced in #1587, so I'm not marking it as breaking.

@aborgna-q aborgna-q requested a review from a team as a code owner October 17, 2024 15:37
@aborgna-q aborgna-q requested a review from doug-q October 17, 2024 15:37
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 88.52459% with 14 lines in your changes missing coverage. Please review.

Project coverage is 85.72%. Comparing base (c5b597d) to head (61aae85).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
hugr-core/src/package.rs 92.47% 5 Missing and 2 partials ⚠️
hugr-cli/src/lib.rs 76.00% 5 Missing and 1 partial ⚠️
hugr-cli/src/mermaid.rs 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #1589    +/-   ##
========================================
  Coverage   85.72%   85.72%            
========================================
  Files         136      136            
  Lines       24783    24891   +108     
  Branches    21719    21827   +108     
========================================
+ Hits        21244    21337    +93     
- Misses       2437     2446     +9     
- Partials     1102     1108     +6     
Flag Coverage Δ
python 92.68% <ø> (ø)
rust 84.74% <88.52%> (+<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.

@aborgna-q aborgna-q changed the title Ab/package checks feat: Ensure packages always have modules at the root Oct 17, 2024
@aborgna-q
Copy link
Collaborator Author

aborgna-q commented Oct 17, 2024

#1587 made some breaking changes. I'll try to fix them in a following PR, and replace them with #[deprecated]

edit: #1591

Copy link
Collaborator

@doug-q doug-q left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I wish PackageOrHugr and Package had impl AsRef<[Hugr]>.

I think we should rename Package::validate to Package::update_validate for consistency.

hugr-core/src/package.rs Outdated Show resolved Hide resolved
@aborgna-q
Copy link
Collaborator Author

I renamed the update method. Note that this is a breaking release, but #1591 will take care of deprecating things.

@hugrbot
Copy link
Collaborator

hugrbot commented Oct 21, 2024

This PR contains breaking changes to the public Rust API.
Please deprecate the old API instead (if possible), or mark the PR with a ! to indicate a breaking change.

cargo-semver-checks summary

--- failure enum_variant_missing: pub enum variant removed or renamed ---

Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely.
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/enum_variant_missing.ron

Failed in:
variant CliError::Package, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-cli/src/lib.rs:47

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/inherent_method_missing.ron

Failed in:
HugrArgs::get_package, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-cli/src/lib.rs:79

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
      ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
     impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.36.0/src/lints/inherent_method_missing.ron

Failed in:
Package::validate, previously in file /home/runner/work/hugr/hugr/BASELINE_BRANCH/hugr-core/src/package.rs:36

@aborgna-q aborgna-q added this pull request to the merge queue Oct 21, 2024
Merged via the queue into main with commit d349eee Oct 21, 2024
21 of 22 checks passed
@aborgna-q aborgna-q deleted the ab/package-checks branch October 21, 2024 11:02
@hugrbot hugrbot mentioned this pull request Oct 21, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 22, 2024
## 🤖 New release
* `hugr`: 0.13.1 -> 0.13.2 (✓ API compatible changes)
* `hugr-core`: 0.13.1 -> 0.13.2 (✓ API compatible changes)
* `hugr-model`: 0.13.1 -> 0.13.2 (✓ API compatible changes)
* `hugr-passes`: 0.13.1 -> 0.13.2 (✓ API compatible changes)
* `hugr-cli`: 0.13.1 -> 0.13.2 (✓ API compatible changes)

<details><summary><i><b>Changelog</b></i></summary><p>

## `hugr`
<blockquote>

##
[0.13.2](hugr-v0.13.1...hugr-v0.13.2)
- 2024-10-22

### Bug Fixes

- Allocate ports on root nodes
([#1585](#1585))

### New Features

- Render function names in `mermaid`/`dot`
([#1583](#1583))
- Add filter_edge_kind to PortIterator
([#1593](#1593))
- make errors more readable with Display impls
([#1597](#1597))
- Ensure packages always have modules at the root
([#1589](#1589))
- Add `Package` definition on `hugr-core`
([#1587](#1587))
</blockquote>

## `hugr-core`
<blockquote>

##
[0.13.2](hugr-core-v0.13.1...hugr-core-v0.13.2)
- 2024-10-22

### Bug Fixes

- Allocate ports on root nodes
([#1585](#1585))

### New Features

- Add `Package` definition on `hugr-core`
([#1587](#1587))
- Render function names in `mermaid`/`dot`
([#1583](#1583))
- Add filter_edge_kind to PortIterator
([#1593](#1593))
- make errors more readable with Display impls
([#1597](#1597))
- Ensure packages always have modules at the root
([#1589](#1589))
</blockquote>

## `hugr-model`
<blockquote>

##
[0.13.2](hugr-model-v0.13.1...hugr-model-v0.13.2)
- 2024-10-22

### New Features

- make errors more readable with Display impls
([#1597](#1597))
</blockquote>

## `hugr-passes`
<blockquote>

##
[0.13.2](hugr-passes-v0.13.1...hugr-passes-v0.13.2)
- 2024-10-22

### New Features

- make errors more readable with Display impls
([#1597](#1597))
</blockquote>

## `hugr-cli`
<blockquote>

##
[0.13.2](hugr-cli-v0.13.1...hugr-cli-v0.13.2)
- 2024-10-22

### New Features

- Add `Package` definition on `hugr-core`
([#1587](#1587))
- Ensure packages always have modules at the root
([#1589](#1589))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/MarcoIeni/release-plz/).
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.

3 participants