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.

Sign up for GitHub

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

Jump to bottom

Removed deprecated processor_ids #6563

Merged
merged 10 commits into from
Apr 26, 2024
Merged

Removed deprecated processor_ids #6563

merged 10 commits into from
Apr 26, 2024

Conversation

JaShom
Copy link
Contributor

@JaShom JaShom commented Apr 14, 2024

Removed deprecated processor_ids in Engine, EngineProgram and EngineClient from issue #6271

@JaShom JaShom requested review from wcourtney, vtomole, cduck, verult and a team as code owners April 14, 2024 00:07
@JaShom
Copy link
Contributor Author

JaShom commented Apr 14, 2024

Sorry for the @'s but could someone review this, please? @verult @cduck @wcourtney @vtomole

wcourtney
wcourtney requested changes Apr 15, 2024
View reviewed changes
Copy link
Collaborator

@wcourtney wcourtney left a comment

Choose a reason for hiding this comment

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

There's some documentation that still refers to processor_ids at https://github.com/quantumlib/Cirq/blob/main/docs/google/engine.md. Mind updating that as well? Thanks!

cirq-google/cirq_google/engine/engine_client.py Outdated Show resolved Hide resolved
cirq-google/cirq_google/engine/engine_client.py Outdated Show resolved Hide resolved
cirq-google/cirq_google/engine/engine_client.py Outdated Show resolved Hide resolved
cirq-google/cirq_google/engine/engine_program.py Outdated Show resolved Hide resolved
cirq-google/cirq_google/engine/engine_program.py Outdated Show resolved Hide resolved
@JaShom JaShom requested a review from wcourtney April 17, 2024 21:02
wcourtney
wcourtney reviewed Apr 17, 2024
View reviewed changes
Copy link
Collaborator

@wcourtney wcourtney left a comment

Choose a reason for hiding this comment

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

Just a couple nits, otherwise LGTM. Thanks!

cirq-google/cirq_google/engine/engine.py Outdated Show resolved Hide resolved
cirq-google/cirq_google/engine/engine_client.py Outdated Show resolved Hide resolved
@JaShom JaShom requested a review from wcourtney April 17, 2024 22:46
@JaShom
Copy link
Contributor Author

JaShom commented Apr 18, 2024

@wcourtney are the changes alright?

wcourtney
wcourtney reviewed Apr 19, 2024
View reviewed changes
Copy link
Collaborator

@wcourtney wcourtney left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! LGTM, but it looks like there's a lint error. Other tests are running now. Please resolve any failures and I'll take another look.

@codecov Codecov
Copy link

codecov bot commented Apr 19, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.78%. Comparing base (e1b03ef) to head (4346bf3).
Report is 2 commits behind head on main.

❗ Current head 4346bf3 differs from pull request most recent head 32a7bb2. Consider uploading reports for the commit 32a7bb2 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #6563      +/-   ##
==========================================
- Coverage   97.79%   97.78%   -0.01%     
==========================================
  Files        1124     1124              
  Lines       95684    95466     -218     
==========================================
- Hits        93574    93352     -222     
- Misses       2110     2114       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JaShom JaShom requested a review from wcourtney April 20, 2024 00:36
@JaShom
Copy link
Contributor Author

JaShom commented Apr 20, 2024 •
edited
Loading

@wcourtney I've resolved all the failures :)

@JaShom
Copy link
Contributor Author

JaShom commented Apr 25, 2024

@wcourtney Is it good now?

@JaShom
Copy link
Contributor Author

JaShom commented Apr 25, 2024

Thanks guys @wcourtney @pavoljuhas

wcourtney
wcourtney approved these changes Apr 26, 2024
View reviewed changes
Copy link
Collaborator

@wcourtney wcourtney left a comment

Choose a reason for hiding this comment

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

Sorry for the delay! LGTM

@wcourtney wcourtney merged commit 2474d47 into quantumlib:main Apr 26, 2024
29 checks passed
jselig-rigetti pushed a commit to jselig-rigetti/Cirq that referenced this pull request May 28, 2024
@JaShom @pavoljuhas @jselig-rigetti
Removed deprecated processor_ids (quantumlib#6563)
c4176e3 
* removed deprecated processor_ids in Engine, EngineProgram and EngineClient

* Made processor_id required, engine.md updated

* Update engine.md

removed the `[]` for processor_id

* processor_id required in Engine, omitted a condition in EngineClient

* Format and lint changes made

---------

Co-authored-by: Pavol Juhas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wcourtney wcourtney wcourtney approved these changes

@vtomole vtomole Awaiting requested review from vtomole vtomole is a code owner

@cduck cduck Awaiting requested review from cduck

@verult verult Awaiting requested review from verult verult is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@JaShom @wcourtney @pavoljuhas