Export rust QCS ClientConfiguration to python #235

jselig-rigetti · 2023-01-19T18:42:22Z

Closes #231

~~Open~~ questions:

The info method helps with testing, but one could mistake the output for being serializable when it isn't really. Any thoughts on if we should provide a serialized view of the client, or what should be inspectable by the client?
Implemented the == operator overload without exposing Debug output.
~~How do we add pyi type stubs for nested submodules? Should everything be exported at the top level?~~ Use https://www.maturin.rs/project_layout.html#mixed-rustpython-project but only have .pyi files.

…Python

crates/python/src/qpu/client.rs

crates/python/tests/qcs_config/settings.toml

MarquessV

According to maturin we want a <module_name>.pyi in the root for each module, but it isn't explicit about what to do for submodules. I would try just the module name first to see if that works. If not, maybe try the full module path? Ie. qcs_sdk.my_module.pyi

crates/python/tests/test_client.py

kalzoo

I don't quite follow this sentence:

The info method helps with testing, but one could mistake the output for being serializable when it isn't really. Any thoughts on if we should provide a serialized view of the client, or what should be inspectable by the client?

Has that already been removed, or am I missing it in review? I think we're okay without serializing the client; it's just a handle on file data anyway (which is a neat serialized format) and there are getters and setters for everything, so I can't think of anything the user would need to do but couldn't.

Overall, this looks good to me, and neat 🗃️ . I'd like to ask for a look from @Shadow53 at the macro invocation for a full approval.

There's also the pyi type hints - I don't have any insight there and would likely just try the same things you are. If nothing else, make sure to clean up any experiments/cruft prior to final review & merge.

Other than that - I'd be happy to approve this.

crates/python/qcs_sdk/qpu/client/__init__.pyi

crates/python/tests/test_client.py

crates/python/src/qpu/client.rs

MarquessV

Minor comment on the type hints, but looks good!

crates/python/qcs_sdk/qpu/client.pyi

MarquessV

🚀

* chore: add helper for listing qpu names * chore: mr feedback * chore: paginate with timeout * chore: comment update * Update crates/python/src/api.rs Co-authored-by: Michael Bryant <mbryant@rigetti.com> * chore: use tokio timeout * chore: better error verbiage * chore: fix imports add test * chore: use namespacing instead of renaming * chore: use error converter helper * chore: remove superfluous gitignore Co-authored-by: Michael Bryant <mbryant@rigetti.com>

Shadow53

The Rust looks good, except for a couple clarifications. There are things to be corrected with the pyi files, though.

I started to comment each instance, but I was basically repeating myself for the same 3-4 things, so I instead left a comment pointing out how a thing should be fixed and then left it as an exercise for the reader to find the others.

I'm open to pairing on this if there are any questions about how the pyi files should be written.

crates/lib/src/api.rs

crates/python/qcs_sdk/__init__.py

crates/python/qcs_sdk/_executable.pyi

crates/python/qcs_sdk/__init__.pyi

crates/python/qcs_sdk/_register_data.pyi

crates/python/qcs_sdk/api.pyi

crates/python/src/qpu/client.rs

crates/python/qcs_sdk/_executable.pyi

…tion in client

crates/python/src/executable.rs

Shadow53

Left a few notes. The big one is that there are types in the pyi files with @property getters but not setters, but I think they get setters from the macros

crates/python/qcs_sdk/_execution_data.pyi

crates/python/src/executable.rs

crates/python/tests/test_api.py

Shadow53

A couple more places that might need setters in pyi, and a question whether a type can be a pyclass instead of a wrapped type.

crates/python/qcs_sdk/api.pyi

crates/python/qcs_sdk/qpu/quilc.pyi

crates/python/src/qpu/client.rs

Shadow53

LGTM

Shadow53 and others added 15 commits January 9, 2023 15:40

refactor(lib): remove unused module

a712312

refactor(lib)!: use num::Complex instead of custom typedef

47bf6e0

chore(lib): remove newly removed lint

8a68b83

refactor(lib)!: use Cow<str> instead of &str for execution

fd97469

style(lib): resolve clippy lints

c2879aa

feat(python): start directly wrapping types from the Rust SDK

aa729d1

refactor(lib)!: replace Box<str> -> String, &str -> Cow<str>

dbbc6e3

feat(python): wrap more types for Python

87a39e1

feat(python): add missing wrappers for remaining types

6ad951b

refactor(python): change how Complex64ReadoutValues converts to/from …

d1cf077

…Python

chore(python): use rigetti-pyo3 from git, not local path

e847da7

feat(python): define default arguments to methods

1ae1298

feat: add more constructor helpers to qpu client

1136799

chore: work in progress testing the qpu client

2e3171f

chore: mr cleanup

b352749

jselig-rigetti commented Jan 19, 2023

View reviewed changes

crates/python/src/qpu/client.rs Show resolved Hide resolved

MarquessV reviewed Jan 19, 2023

View reviewed changes

crates/python/src/qpu/client.rs Outdated Show resolved Hide resolved

jselig-rigetti commented Jan 19, 2023

View reviewed changes

crates/python/tests/qcs_config/settings.toml Show resolved Hide resolved

MarquessV reviewed Jan 19, 2023

View reviewed changes

crates/python/tests/test_client.py Outdated Show resolved Hide resolved

jselig-rigetti added 3 commits January 19, 2023 13:31

chore: use nested pyi files

bc18576

chore: clean up imports and declarations

3461ee0

chore: impl eq through __richcmp__

e6fbbd1

jselig-rigetti marked this pull request as ready for review January 20, 2023 22:48

kalzoo reviewed Jan 21, 2023

View reviewed changes

crates/python/qcs_sdk/qpu/client/__init__.pyi Outdated Show resolved Hide resolved

crates/python/tests/test_client.py Show resolved Hide resolved

crates/python/src/qpu/client.rs Show resolved Hide resolved

jselig-rigetti and others added 3 commits January 23, 2023 10:38

chore: type stubs for all exports

f0fc5b7

doc(lib): explain seeming type disagreement

ef9a28e

refactor(python): simplify code, remove TODO

5600581

jselig-rigetti mentioned this pull request Jan 23, 2023

feat!: implement Python wrappers for the full Rust API #230

Merged

MarquessV reviewed Jan 23, 2023

View reviewed changes

crates/python/qcs_sdk/qpu/client.pyi Show resolved Hide resolved

fix: typedict base class

7ef8bda

Shadow53 and others added 5 commits January 23, 2023 17:07

chore(python): gitignore __pycache__

b9c91f3

chore(lib): update tokio, disable unused warp features

2babc4d

Merge branch '208-return-pyo3-types' into 231-qcs-client-config

01bf1d9

chore: add import

e921666

fix: add methods for variants

f57b65f

MarquessV approved these changes Jan 24, 2023

View reviewed changes

Base automatically changed from 208-return-pyo3-types to main January 24, 2023 19:29

jselig-rigetti and others added 3 commits January 24, 2023 12:31

Merge branch 'main' into 231-qcs-client-config

f5b819f

chore: ignore api-bound pytest

ac2d5ea

kalzoo approved these changes Jan 24, 2023

View reviewed changes

Shadow53 requested changes Jan 24, 2023

View reviewed changes

jselig-rigetti added 4 commits January 25, 2023 10:55

chore(python): update pyi definitions and use better struct serializa…

e3595c1

…tion in client

chore(python): add pyo3 usage tests

1da3a51

chore(python): update return type definition for executable methods

a5b06c7

chore: make all attributes into properties

0977032

jselig-rigetti commented Jan 26, 2023

View reviewed changes

crates/python/src/executable.rs Outdated Show resolved Hide resolved

jselig-rigetti mentioned this pull request Jan 27, 2023

Allow all API calls to specify a timeout #239

Open

jselig-rigetti added 2 commits January 27, 2023 10:52

chore: make default timeout a static value

a146bb6

chore: use fixed version of rigetti-pyo3

09cbd5e

Shadow53 requested changes Jan 27, 2023

View reviewed changes

crates/python/qcs_sdk/_execution_data.pyi Show resolved Hide resolved

crates/python/src/executable.rs Outdated Show resolved Hide resolved

crates/python/src/executable.rs Outdated Show resolved Hide resolved

crates/python/tests/test_api.py Outdated Show resolved Hide resolved

jselig-rigetti added 4 commits January 27, 2023 11:55

fix: mr feedback

8300983

fix: link formatting in comment

a484284

fix: use pyclass instead of wrappers

db1c96e

fix: remove needless lifetime, per ci

b93dcd4

Shadow53 requested changes Jan 27, 2023

View reviewed changes

crates/python/qcs_sdk/api.pyi Show resolved Hide resolved

crates/python/qcs_sdk/qpu/quilc.pyi Show resolved Hide resolved

crates/python/src/qpu/client.rs Outdated Show resolved Hide resolved

fix: mr suggestions

c095c59

Shadow53 approved these changes Jan 27, 2023

View reviewed changes

jselig-rigetti merged commit 39c878f into main Jan 27, 2023

jselig-rigetti deleted the 231-qcs-client-config branch January 27, 2023 23:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Export rust QCS ClientConfiguration to python #235

Export rust QCS ClientConfiguration to python #235

jselig-rigetti commented Jan 19, 2023 •

edited

MarquessV left a comment •

edited

kalzoo left a comment

MarquessV left a comment

MarquessV left a comment

Shadow53 left a comment

Shadow53 left a comment

Shadow53 left a comment

Shadow53 left a comment

Export rust QCS ClientConfiguration to python #235

Export rust QCS ClientConfiguration to python #235

Conversation

jselig-rigetti commented Jan 19, 2023 • edited

MarquessV left a comment • edited

Choose a reason for hiding this comment

kalzoo left a comment

Choose a reason for hiding this comment

MarquessV left a comment

Choose a reason for hiding this comment

MarquessV left a comment

Choose a reason for hiding this comment

Shadow53 left a comment

Choose a reason for hiding this comment

Shadow53 left a comment

Choose a reason for hiding this comment

Shadow53 left a comment

Choose a reason for hiding this comment

Shadow53 left a comment

Choose a reason for hiding this comment

jselig-rigetti commented Jan 19, 2023 •

edited

MarquessV left a comment •

edited