-
Notifications
You must be signed in to change notification settings - Fork 194
Move setup.py metadata into pyproject.toml, remove dev-requirements.txt, & fix #839
#985
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
Open
mhucka
wants to merge
35
commits into
quantumlib:main
Choose a base branch
from
mhucka:mh-pyproject
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+181
−131
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The modern approach is to put more things into `pyproject.toml`. This moves most things out of `setup.py`. The version number is now read from from the file `qsimcirq/_version.py` in both setup.py and pyproject.toml, so that there is a single source of truth for that value.
Remove no-longer-needed dev-requirements.txt. Its contents are in pyproject.toml now.
The more modern way of handling development dependencies is to put them in pyproject.toml and then use `pip install --group dev` to install them.
Some transitive dependencies have had version updates that cause conflicts for some combinations of our builds, such as contourpy requiring Python 3.11. This puts more constraints on this so that qsimcirq builds everywhere.
1. Need to change the workdir at a different point. 2. Need to activate the venv.
pavoljuhas
requested changes
Jan 10, 2026
Collaborator
pavoljuhas
left a comment
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.
If possible, cmake and pybind should be dropped from runtime dependencies (see inline). Otherwise LGTM with some optional suggestions.
This updates some of the jobs in the CI workflow to use the larger runners we've configured for our org, so that CI runs faster.
This is a simple script to help contributors easily run pylint recursively on the right directories. It also passes `--jobs=0` to make it as fast as possible by default.
A better practice for jobs where the first step is to `cd` into a directory is to use the field `working-directory` in the step definition and skip the `cd`.
…user know (quantumlib#967) Due to the fact that the package name is not the same as the module name (`py-cpuinfo` vs `cpuinfo`), the error that results from trying to run this script when `py-cpuinfo` is not installed can be confusing. Let's help users & developers by testing for the package and pointing them in the right direction. Also, slightly improve the clarity of the usage message text in this script. --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
On systems where there is no /proc/cpuinfo, you get an error message. We don't need to see it; we just need the result of the grep.
Tell Homebrew not to auto-update when running. Auto-updating is not necessary here and just wastes time when it happens.
IMHO it's useful to see Markdown files as one of the types of files listed on the GitHub front page in the file statistics section, alongside other files. It gives a sense for the amount of documentation present in a project. On the other hand, some configuration files are not useful to count, such as .md files in a `.gemini/` subdirectory and configuration files for tools like git.
`isort` is used in `check/format-incremental`, but there is no configuration for it. This adds a configuration based on what Cirq currently uses.
The warning described in quantumlib#929 seems to be the result of using an older version of Eigen. Updating the version makes the warning go go away.
…umlib#990) The way `CXXFLAGS` and other flags were being set meant that users had to add the default values (`-std=c++17 -fopenmp -O3` etc) if they set the flags at all, or else lose the defaults. This was error-prone and suboptimal. The change here makes it so that the user values are appended at the end of the defaults, so that they can override values. This PR also adds a DEBUG flag that switches between using `-g -O0` and the regular `-O3` option. Developers can invoke it using, e.g., `make DEBUG=1`.
…ntumlib#991) There was an inconsistency in the Makefiles versus `tests/BUILD` and `tests/make.sh`, in that a couple of the Makewfiles used `-msse4.1` while the BUILD file and `tests/make.sh` used `-msse4`. In addition, it seems that `-msse4` subsumes `-msse4.1`, and so overall, it appears better to use `-msse4`. Fixes quantumlib#892.
This adds checks to the GitHub Actions workflows for the owner being Quantumlib, so that users who fork the repo will not be surprised by the workflows running in their forks.
It looks like it's possible to use the `-mbmi2` flag when `-mavx2` is used, based on documentation and on testing locally. This was done in some Makefiles but not consistently. This PR adds the flag to Makefiles where it was missing, plus also in one BUILD file.
…mlib#995) The `CMakeBuild` class' `run()` method tests the version of CMake; however, it did the test only when on Windows. It seems like this is a test worth doing everywhere, so I removed test for Windows. In addition, while at it, I slightly updated the way the process output is captured, and expanded the range of exceptions tested in order to provide more specific feedback to users. --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
As pointed out by Pavol in the review comments, CMake should only be needed for building qsim, and not a run-time installation dependency.
Move the installation of the dev dependencies to the top-level Dockerfile because things fail to build otherwise.
As noted by Pavol in https://github.com/quantumlib/qsim/pull/985/changes#r2678037600, it's not necessary to update pip when `--upgrade-deps` is used in the venv creation.
As pointed out by Pavol in review comments, the venv activation command was pointless.
Setting the workdir makes more sense to do before creating the venv.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This addresses deprecation warnings originating from the use of outdated
setuptoolsfunctions, and takes the opportunity to modernize the management of the version number and the dependencies in accordance with current Python practices (PEP 621, 735).Key changes include:
All declarative package metadata (e.g., name, author, classifiers) has been moved from
setup.pytopyproject.toml. What's left insetup.pyhas been simplified to only contain the logic necessary for building the C++ extensions and callingsetup.The qsim version number is now stored in only one file,
qsimcirq/_version.py, and bothpyproject.tomlandsetup.pyread it from there. In the case ofsetup.py, the use of the deprecatedself.distribution.get_version()has been replaced with the use ofrunpyto read the version number fromqsimcirq/_version.py. (The latter change fixes Fix use of deprecated setuptools functions #839.)The development dependencies are no longer stored as "extras" but rather use the pyproject.toml
[dependency-groups]section introduced in 2024 by PEP 735 and recognized by pip version 25.1+. This meansdev-requirements.txtis gone, and development dependencies are installed usingpyproject.tomldefines the versions of Python for which cibuildwheel builds wheels (in section[tool.cibuildwheel]).