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

refactor: split dev requirements out of main requirements.txt #467

Merged

Conversation

lovesegfault
Copy link
Contributor

Currently, if you try to use a tool like Poetry to specify a dependency on
qsim you end up with the unexpected behavior of your package's version of
black having to be pinned to same version qsim pins in its requirements.txt.

This should solve this by splitting development dependencies out of the main dependency file.

@google-cla google-cla bot added the cla: yes Override CLA status to unblock PR. label Oct 30, 2021
@95-martin-orion
Copy link
Collaborator

This change will likely also require updates to existing documentation to ensure that developers know to install the additional requirements file manually, and to the CI workflows so that tests will pass.

@95-martin-orion 95-martin-orion self-requested a review November 1, 2021 14:44
@lovesegfault lovesegfault force-pushed the split-dev-dependencies branch 6 times, most recently from e83b7f3 to 31e3b72 Compare November 1, 2021 19:00
@lovesegfault
Copy link
Contributor Author

This change will likely also require updates to existing documentation to ensure that developers know to install the additional requirements file manually, and to the CI workflows so that tests will pass.

Done! I think this is ready for review now :)

@lovesegfault lovesegfault force-pushed the split-dev-dependencies branch 2 times, most recently from eee7543 to 92e6c49 Compare November 4, 2021 17:54
@95-martin-orion
Copy link
Collaborator

Code changes all LGTM. Do you know if this will play nicely with our existing notebooks, though? I have a suspicion that moving the pybind11 dependency to a separate package might break things when pip-installing.

@lovesegfault
Copy link
Contributor Author

Code changes all LGTM. Do you know if this will play nicely with our existing notebooks, though? I have a suspicion that moving the pybind11 dependency to a separate package might break things when pip-installing.

People will have to do pip install qsimcirq[dev] now to get all the deps. Happy to test this if you can show me how.

@95-martin-orion
Copy link
Collaborator

Testing is somewhat inconvenient, given that it requires a qsimcirq release (preferably on test-pypi). Instead, if black and flynt are the main concerns here, could you shift pybind11 from the dev requirements back into the main requirements? That should resolve my concerns so tests aren't necessary.

@lovesegfault lovesegfault force-pushed the split-dev-dependencies branch from 92e6c49 to f0491cc Compare November 4, 2021 20:23
@lovesegfault
Copy link
Contributor Author

Testing is somewhat inconvenient, given that it requires a qsimcirq release (preferably on test-pypi). Instead, if black and flynt are the main concerns here, could you shift pybind11 from the dev requirements back into the main requirements? That should resolve my concerns so tests aren't necessary.

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Override CLA status to unblock PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants