feat: allow direct endpoint connection strategy#573
feat: allow direct endpoint connection strategy#573jselig-rigetti wants to merge 7 commits intomainfrom
Conversation
|
Although I'm not exactly certain what's going wrong for you, I suspect you'll be able to resolve it by deleting and recreating your virtualenv. Assuming you have Python 3.10+ on the path, you can do that by running this from the project's
You can run The tasks that run Python use The other tasks related to Python bindings typically depend on the |
asaites
left a comment
There was a problem hiding this comment.
Overall, this looks good to me. It needs the Python stubs regenerated, and I understand you had some trouble with that. I also had some suggestions about documentation and naming, but they're not functional changes. With the stubs updated, I'm happy for it to merge.
|
Ah @asaites thanks, got it working with the following: PYTHONHOME=$(uv python dir)/cpython-3.12.12-macos-aarch64-none makers generate-stubs |
Co-authored-by: Alexander Saites <215926835+asaites@users.noreply.github.com>
Co-authored-by: Alexander Saites <215926835+asaites@users.noreply.github.com>
| from . import qvm | ||
|
|
||
| __version__: typing.Final = '0.25.18-rc.0' | ||
| __version__: typing.Final = '0.26.0-rc.0' |
There was a problem hiding this comment.
I guess this isn't being changed on knope release
There was a problem hiding this comment.
Ah, yeah, I hadn't though about that. The actual value of __version__ is determined from CARGO_PKG_VERSION when the extension module is built, so it technically only matches the stub value if the stubs are regenerated after the package version is set. Rather than having knope regenerate the stubs, it's probably more sensible to just elide the specific value itself. That requires a little bit of type trickery, so I'll open a separate issue/PR for it, if that's alright with you.
There was a problem hiding this comment.
There was a problem hiding this comment.
Other comments (2)
- crates/lib/src/qpu/api/python.rs (738-738) There's an inconsistency in the documentation. The parameter description mentions `ConnectionStrategy.endpoint_address()` but based on other documentation in the file, it should be `ConnectionStrategy.direct_endpoint_address()`.
- crates/lib/src/qpu/api/mod.rs (572-577) The `EndpointAddress` strategy doesn't require a quantum processor ID for connection, but it still needs one when using the target methods (`get_job_target`, `get_results_target`, `get_cancel_target`). Consider adding a validation check in these methods to ensure a quantum processor ID is provided when needed.
💡 To request another review, post a new comment with "/windsurf-review".
| # NOTE: if using `uv`, you may need to set `PYTHONHOME` to the root of the virtual environment: | ||
| # | ||
| # export PYTHONHOME=$(python3 -c "import sys; print(sys.base_prefix)") |
There was a problem hiding this comment.
I'm very surprised this would be the case. I don't have that variable set in my environment, but since the resolution is managed by PyO3, there must be something about our environments that are different. I'm glad you found a resolution that worked for you, though.
Closes #572
I think I may have my python env wrong (using
uv venv) but this is what I get frommakers generate-stubs