Skip to content

Feature/issue 1333 pyproject modernize - #1381

Open
rosspeili wants to merge 3 commits into
quantumlib:mainfrom
rosspeili:feature/issue-1333-pyproject-modernize
Open

Feature/issue 1333 pyproject modernize#1381
rosspeili wants to merge 3 commits into
quantumlib:mainfrom
rosspeili:feature/issue-1333-pyproject-modernize

Conversation

@rosspeili

@rosspeili rosspeili commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Moved project metadata from setup.py into pyproject.toml (PEP 621), modeled on qsim: [project] metadata, dynamic version/deps from dev_tools/requirements/deps/*.txt, resources optional extra, and a PEP 735 dev dependency group for uv sync --group dev. Removed setup.py, and updated MANIFEST.in and _version.py accordingly.

Tested with python -m build (sdist + wheel), pip install -e ., and full pytest (2196 passed; 6 Windows-only parallel-test failures also on main), also updated CONTRIBUTING.md to point at pyproject.toml as the canonical config and document the uv sync --group dev workflow.

Intentionally deferred:

  • Migrating .pylintrc / other dotfile tool configs into pyproject.toml
  • Adding a committed uv.lock (if we want that workflow?)
  • Changing CI to use uv sync instead of locked dev_tools/requirements/envs/*.txt (those files and create-env-files.sh are unchanged)

Closes #1333

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request migrates the project's build configuration and metadata from setup.py to pyproject.toml, updating documentation, packaging scripts, and manifest files accordingly. Feedback on the changes suggests correcting the license field format in pyproject.toml to comply with PEP 621 and restoring the missing Apache Software License classifier to maintain parity with the previous setup.

Comment thread pyproject.toml
Comment thread pyproject.toml
@rosspeili

Copy link
Copy Markdown
Contributor Author

@mhucka keeping license = "Apache-2.0" (SPDX, PEP 639) per qsim. Adding the License trove classifier back causes setuptools to fail the build (License classifiers have been superseded by license expressions). Verified with python -m build. Should I ignore Gemini assist's recommendations?

@mhucka

mhucka commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

@mhucka keeping license = "Apache-2.0" (SPDX, PEP 639) per qsim. Adding the License trove classifier back causes setuptools to fail the build (License classifiers have been superseded by license expressions). Verified with python -m build. Should I ignore Gemini assist's recommendations?

Thank you for that work!

Re GCA's comments about the license field: it's wrong. PEP 621 has been superceded by PEP 639 and (as you noted) that one says string values are preferred. Re GCA's comments about adding the classifier: let's ignore that one too, if setuptools doesn't work with it.

@mhucka

mhucka commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Regarding the timing of merging this PR: we're trying to get a release out this week, so we're going to leave this until after the release.

@rosspeili

Copy link
Copy Markdown
Contributor Author

Thanks @mhucka, glad this looks on track. No stress from my side, happy to wait until after the release. Ping me if anything else should change before merge. <3

@mhucka mhucka added priority/after-1.8.1 area/devops Involves build systems, Make files, Bazel files, continuous integration, and or other DevOps topics and removed priority/after-1.7.2 labels Jul 14, 2026
@mhucka mhucka self-assigned this Aug 13, 2026

@mhucka mhucka left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for taking this on, and sorry for the long delay in getting back to this PR.

This looks good. I have just a couple of fussy change requests.

Before merging this, I'd like to merge #1445 first, to update the version of Pylint; once the other PR is merged, if you could update the version of pylint in this pyproject.toml, the result should help ensure everything works together.

Comment thread pyproject.toml Outdated
Comment thread CONTRIBUTING.md Outdated
)

Model packaging on qsim: PEP 621 [project] metadata, setuptools dynamic version and dependencies from requirement files, optional resources extra, and a PEP 735 dev dependency group for uv. Remove setup.py; keep locked CI env files via create-env-files.sh.
Add resource-estimate deps to dev group for uv parity. Document pyproject.toml and uv sync in CONTRIBUTING.
…t 4.0

Restore the existing copyright year, drop the parenthetical about setup.py, pin pylint to 4.0 to match quantumlib#1445, and pick up the package description and pytest helper deps from main.
@rosspeili
rosspeili force-pushed the feature/issue-1333-pyproject-modernize branch from c510879 to a51907b Compare August 18, 2026 06:01
@rosspeili

Copy link
Copy Markdown
Contributor Author

Nw at all @mhucka, and thanks for the review. Copyright restored and won't touch again, explainer referencing non existing setup.py dropped, and pylint~=4.0 is already in pyproject.toml so this should line up once #1445 lands. Lmk how this looks <3

@mhucka

mhucka commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

@rosspeili Other PR merged. Once you update the branch on this one, I think it'll be ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/devops Involves build systems, Make files, Bazel files, continuous integration, and or other DevOps topics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move setup.py metadata and as many tool configurations into pyproject.toml as possible

2 participants