-
Notifications
You must be signed in to change notification settings - Fork 354
fix: ensure pragma extern maps are copied #1819
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
Conversation
6513e71
to
e44c89e
Compare
e44c89e
to
f25dd7d
Compare
|
Branch | support_extern_call_instructions |
Testbed | ci-runner-linux |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholds
flag.
Click to view all benchmark results
Benchmark | Latency | seconds (s) |
---|---|---|
test/benchmarks/test_program.py::test_copy_everything_except_instructions | 📈 view plot | 10.97 s |
test/benchmarks/test_program.py::test_instructions | 📈 view plot | 4.06 s |
test/benchmarks/test_program.py::test_iteration | 📈 view plot | 4.33 s |
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
2daf801
to
5a291ea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The quil-rs contentful changes look good to me, but somebody with a better understanding of Python and/or CI needs to review the CI changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far - do you think you could add some tests cases to https://github.com/rigetti/pyquil/blob/master/test/unit/test_quilbase.py to ensure that data roundtripping (from rust) and pickling work as expected?
c11addf
to
3385518
Compare
@jselig-rigetti Thanks for the review. I've addressed all of your comments. |
Description
This PR essentially ensures that quil's
Program.extern_pragma_map
is properly handled in pyQuil: https://github.com/rigetti/quil-rs/blob/53ce859d61a86383c4d98dcd4033d9cd375a5937/quil-rs/src/program/mod.rs#L86Namely, those pragma instructions should be copied when appropriate.
Also, there's quite a bit of CI tire kicking here: I've added
pip install --upgrade pip
in a few different places, added the condition marks mentioned above, bumped python in the docker image to 3.10, updated poetry to 1.8.5, and used poetry to build the wheel in CI rather than pip.Checklist
master
branch