Fix #839 and move most setup.py metadata and config into pyproject.toml
#959
Conversation
The modern approach is to put more things into `pyproject.toml`. This moves most things out of `setup.py`, with the exception of the native code extensions parts, and the version number. (Still to do: figure out how to move the version number too.)
mhucka
force-pushed
the
mh-more-pyproject
branch
from
4242c63 to
4823984
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modernizes the project's packaging by migrating metadata from setup.py to pyproject.toml in line with PEP 621. The changes are well-executed and correctly move dependencies, project information, and other configurations. My main feedback is to eliminate the duplication of the version string, which is currently present in both pyproject.toml and setup.py. By fetching the version from the distribution metadata within setup.py, you can make pyproject.toml the single source of truth for the version, which improves maintainability and prevents potential inconsistencies.
mhucka
changed the title
setup.py configuration and metadata into pyproject.toml
mhucka
changed the title
setup.py configuration and metadata into pyproject.tomlsetup.py metadata and config into pyproject.toml
| "absl-py", | ||
| "cirq-core~=1.0", | ||
| "numpy>=1.26.0", |
There was a problem hiding this comment.
This duplicates requirements.txt - do you plan on retiring that file together with dev-requirements.txt and only declare dependencies in pyproject.toml?
You might want to check if qsim can be converted to a uv managed project which will support dependency-tree locking and a full development reproducibility.
| from setuptools import Extension, setup | ||
| from setuptools.command.build_ext import build_ext | ||
|
|
||
| __version__ = "0.23.0.dev0" |
There was a problem hiding this comment.
You can use tomllib to get version from pyproject.toml and avoid duplicate definition. That modules is builtin since Python 3.11
pavoljuhas
left a comment
pavoljuhas
left a comment
There was a problem hiding this comment.
LGTM after addressing inline comments.
Have you tested if wheel building - especially with cibuildwheel - works with these changes?
2 participants
This addresses deprecation warnings originating from the use of outdated
setuptoolsfunctions. I took this opportunity to modernize the package definition in accordance with current Python packaging standards (PEP 621).Key changes include:
setup.pytopyproject.toml.setup.pyfile has been simplified to only contain the logic necessary for building the C++ extensions.self.distribution.get_version()method has been replaced with a direct use of the__version__variable.py.typedmarker file is now correctly included in the package via the[tool.setuptools.package-data]configuration inpyproject.toml.This should fix #839 and also set the stage for using
pyproject.tomlto add more metadata and project configuration settings.(I used Jules to help with this.)