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.

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

Loosen pyquil requirements #5681

Merged
merged 7 commits into from
Jul 11, 2022
Merged

Loosen pyquil requirements #5681

merged 7 commits into from
Jul 11, 2022

Conversation

vtomole
Copy link
Collaborator

@vtomole vtomole commented Jul 7, 2022

No description provided.

@CirqBot CirqBot added the Size: XS <10 lines changed label Jul 7, 2022
@CirqBot CirqBot added the size: S 10< lines changed <50 label Jul 8, 2022
@vtomole
Copy link
Collaborator Author

vtomole commented Jul 8, 2022

Hey @erichulburd, do you recall why you pinned so many packages here instead of inheriting what's giving from pyquil?

@vtomole vtomole changed the title Update pyquil version Loosen pyquil requirements Jul 8, 2022
@vtomole vtomole marked this pull request as ready for review July 8, 2022 00:14
@vtomole vtomole requested review from a team and cduck as code owners July 8, 2022 00:14
@vtomole vtomole requested a review from viathor July 8, 2022 00:14
@vtomole vtomole changed the title Loosen pyquil requirements Loosen pyquil requirements Jul 8, 2022
Copy link
Collaborator

@dstrain115 dstrain115 left a comment

Choose a reason for hiding this comment

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

What's with the other deletions? Are they dependencies of pyquil? Are we sure we can delete them?

@vtomole
Copy link
Collaborator Author

vtomole commented Jul 8, 2022

What's with the other deletions?

All the deleted packages are installed during the pyquil installation:

Are they dependencies of pyquil?

Yes

Are we sure we can delete them?

I think so. The pyquil docs only tell us to pip install pyquil if we want to install pyquil. It excludes all this other stuff. Pinging @erichulburd @kalzoo @dbanty for good measure.

@vtomole vtomole added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 11, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 11, 2022
@CirqBot CirqBot merged commit 7decd91 into master Jul 11, 2022
@vtomole vtomole deleted the vtomole-patch-1-1 branch July 11, 2022 17:58
@erichulburd
Copy link
Contributor

erichulburd commented Jul 11, 2022

Hey @erichulburd, do you recall why you pinned so many packages here instead of inheriting what's giving from pyquil?

I do not recall exactly, but since dependencies installed and tests passed, I'm fairly confident there won't be an issue.

I vaguely remember an update on qcs-api-client being necessary early in the development cycle. My guess is that those dependencies are an artifact of that and I never circled back around to clean them up.

rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. size: S 10< lines changed <50 Size: XS <10 lines changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants