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: Add Package definition on hugr-core #1587

Merged
merged 3 commits into from
Oct 17, 2024
Merged

Conversation

aborgna-q
Copy link
Collaborator

Moves the definition of packages out of hugr-cli. (We leave a re-export there to avoid breaking the API, it should be removed on the next breaking release).

Adds some helper functions to load and validate the package.

Closes #1530

drive-by: Replace hugr-core dependency on hugr-cli by hugr. There's no need to access the internals there.

@aborgna-q aborgna-q requested a review from a team as a code owner October 16, 2024 16:10
@aborgna-q aborgna-q requested a review from zrho October 16, 2024 16:10
Copy link

codecov bot commented Oct 16, 2024

Codecov Report

Attention: Patch coverage is 71.08434% with 24 lines in your changes missing coverage. Please review.

Project coverage is 85.75%. Comparing base (2412599) to head (cfadec3).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
hugr-core/src/package.rs 54.00% 21 Missing and 2 partials ⚠️
hugr-cli/src/validate.rs 85.71% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1587      +/-   ##
==========================================
- Coverage   85.79%   85.75%   -0.04%     
==========================================
  Files         135      136       +1     
  Lines       24687    24746      +59     
  Branches    21623    21682      +59     
==========================================
+ Hits        21180    21221      +41     
- Misses       2405     2423      +18     
  Partials     1102     1102              
Flag Coverage Δ
python 92.68% <ø> (ø)
rust 84.77% <71.08%> (-0.05%) ⬇️

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.

Comment on lines +127 to +131
btree_map::Entry::Occupied(mut prev) => {
if prev.get().version() < extension.version() {
*prev.get_mut() = extension.clone();
}
Ok(prev.into_mut())
Copy link
Member

Choose a reason for hiding this comment

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

doesn't appear to be tested

Copy link
Member

@ss2165 ss2165 left a comment

Choose a reason for hiding this comment

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

📦

@aborgna-q
Copy link
Collaborator Author

I added some to_json methods too, to keep things symmetric.

Note that we are not enforcing that packages must only contain module-rooted HUGRs.
I'll make a separate PR with that, as we need to fix hugr-cli once that starts getting checked.

@aborgna-q aborgna-q added this pull request to the merge queue Oct 17, 2024
Merged via the queue into main with commit d899bd3 Oct 17, 2024
20 of 21 checks passed
@aborgna-q aborgna-q deleted the ab/package-on-core branch October 17, 2024 14:19
This was referenced Oct 17, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 21, 2024
Followup to #1587.

Ensures that rust `Package`s 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.
github-merge-queue bot pushed a commit that referenced this pull request Oct 21, 2024
Replaces the breaking changes introduced in #1587 with non-breaking
deprecations.
We should be able to remove these on the next breaking release.
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.

Move Package out of hugr-cli
2 participants