Skip to content

Conversation

@ss2165
Copy link
Member

@ss2165 ss2165 commented Nov 11, 2025

Closes #2671

Goals

  1. Have hugr cli available in environment when installed via uv (uvx hugr --help)
  2. Allow use of cli functionality from python package without spawning subprocess

Implications

  • Either ship binary with wheel or bind the function directly - chose the latter as it is more flexible and avoids a bunch of build faff
  • CLI uses argc + stdin for input and stdout for output (by default), we need a way to do this via binding i/o to avoid subprocess

Primary outcomes

  • Cli invocation bound via pyo3 to python "package script" with same name as binary, meeting goal 1
  • hugr-cli refactored to read inputs from a reader and write outputs to a writer when appropriate, satisfying goal 2. Uses an "override" pattern - when i/o is specified it overrides the corresponding cli-arg

Secondary outcomes

  • CLI main.rs contents moved to lib.rs, allowing use by library dependants
  • Make tracing a default feature for hugr-cli (not used by hugr-py)
  • cli module in python to wrap core sub-commands and parameters, with simply typed interfaces
  • Python test cli use replaced with bound function calls - meaning no need to build cli before running python tests
  • Model loading in python via conversion to json uses cli convert function rather than dedicated binding
  • Dedicated cargo profile for python wheel for smaller wheels (very open to critique on this)
  • Pydantic models to parse describe json output
  • Remove parallel pytest since it is now faster without it

Possible issues

  • Versioning: the cli version is pinned to the rust crates, the python package is not. Meaning uvx hugr --version will report a different value to uv --with hugr run python -c "import hugr; print(hugr.__version__)". Are we ok with this?

@ss2165 ss2165 requested a review from a team as a code owner November 11, 2025 18:03
@ss2165 ss2165 requested a review from cqc-alec November 11, 2025 18:03
@codecov
Copy link

codecov bot commented Nov 11, 2025

Codecov Report

❌ Patch coverage is 71.86544% with 92 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.41%. Comparing base (5e354b9) to head (2c3aebf).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
hugr-cli/src/mermaid.rs 36.11% 20 Missing and 3 partials ⚠️
hugr-cli/src/describe.rs 75.86% 7 Missing and 14 partials ⚠️
hugr-cli/src/lib.rs 77.27% 20 Missing ⚠️
hugr-cli/src/hugr_io.rs 37.50% 14 Missing and 1 partial ⚠️
hugr-cli/src/convert.rs 57.14% 3 Missing and 3 partials ⚠️
hugr-cli/src/validate.rs 55.55% 4 Missing ⚠️
hugr-py/src/hugr/cli.py 95.45% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2677      +/-   ##
==========================================
- Coverage   83.46%   83.41%   -0.05%     
==========================================
  Files         262      263       +1     
  Lines       51297    51489     +192     
  Branches    46858    46984     +126     
==========================================
+ Hits        42813    42949     +136     
- Misses       6105     6156      +51     
- Partials     2379     2384       +5     
Flag Coverage Δ
python 91.52% <95.58%> (+0.05%) ⬆️
rust 82.63% <65.63%> (-0.07%) ⬇️

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.

@ss2165 ss2165 force-pushed the ss/push-qmnzpxnuzzks branch from 169afd7 to 195070d Compare November 11, 2025 18:14
Copy link
Collaborator

@cqc-alec cqc-alec left a comment

Choose a reason for hiding this comment

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

Looks good, just a few comments.

Comment on lines 223 to 225
format: Target format ("json" or "model", default: auto-detect).
text: Output as text (default: auto-detect).
binary: Output as binary (default: auto-detect).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not clear what "auto-detect" means for output format. (I think the default is to use the input format, i.e. do no conversion, if not specified?)

text: Output as text (default: auto-detect).
binary: Output as binary (default: auto-detect).
compress: Compress the output (default: False).
compression_level: Compression level 0-9 (default: 6).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be 1--22? And where does 6 come from?

Copy link
Member Author

Choose a reason for hiding this comment

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

ah this was the last item on my todo list to fix sorry missed it (is a placeholder)

}
}

fn run_external(args: Vec<OsString>) -> Result<()> {
Copy link
Member Author

Choose a reason for hiding this comment

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

moved unchanged to lib.rs

text: Output as text (default: auto-detect).
binary: Output as binary (default: auto-detect).
compress: Compress the output (default: False).
compression_level: Compression level 0-9 (default: 6).
Copy link
Member Author

Choose a reason for hiding this comment

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

ah this was the last item on my todo list to fix sorry missed it (is a placeholder)

@ss2165 ss2165 requested a review from cqc-alec November 12, 2025 14:52
Comment on lines +65 to +66
/// Directly invoke the HUGR CLI entrypoint.
fn run_cli() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add that we're invoking the CLI passing std::env::args to the docs

Copy link
Collaborator

@aborgna-q aborgna-q Nov 12, 2025

Choose a reason for hiding this comment

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

It may be a bit confusing to call this module cli when including it in the python lib.

From a user standpoint, there's no connection between hugr.cli.describe or hugr.cli.mermaid and a command line interface.

Copy link
Member Author

Choose a reason for hiding this comment

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

Any suggestions?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I sort of agree, but not sure what else to call it. utils is too broad.

Copy link
Member Author

Choose a reason for hiding this comment

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

I personally like making it clear that it relies on the cli interface, it makes the idiosyncrasies of the interface easier to understand (e.g. why pass bytes?). The module docstring explains why it is this way.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was imagining that we would add user level features in future that would wrap around this (e.g. Package.describe() -> PackageDesc) where there is then no reference to the cli.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, let's go with cli for now then

@ss2165 ss2165 added this pull request to the merge queue Nov 12, 2025
Merged via the queue into main with commit 0fd0332 Nov 12, 2025
29 of 30 checks passed
@ss2165 ss2165 deleted the ss/push-qmnzpxnuzzks branch November 12, 2025 16:32
This was referenced Nov 12, 2025
ss2165 added a commit that referenced this pull request Nov 12, 2025
github-merge-queue bot pushed a commit that referenced this pull request Nov 12, 2025
github-merge-queue bot pushed a commit that referenced this pull request Nov 13, 2025
🤖 I have created a release *beep* *boop*
---


##
[0.14.2](hugr-py-v0.14.1...hugr-py-v0.14.2)
(2025-11-13)


### Features

* **cli, python:** programmatic interface to cli with python bindings
([#2677](#2677))
([0fd0332](0fd0332))
* ComposablePass protocol and ComposedPass for hugr-py (unstable)
([[#2636](https://github.com/CQCL/hugr/issues/2636)](https://github.com/CQCL/hugr/pull/2636))
([45dc3fc](45dc3fc))
* return description output to python on error
([#2681](#2681))
([f483146](f483146))
* track package descriptions when loading
([#2639](#2639))
([349dd61](349dd61))


### Documentation

* Fix typo in docstring.
([#2656](#2656))
([a1ce622](a1ce622))

---
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: Seyon Sivarajah <[email protected]>
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.

Distribute cli with python wheel

4 participants