Move some metadata from setup.py to pyproject.toml#37
Move some metadata from setup.py to pyproject.toml#37chaoming0625 merged 11 commits intochaobrain:mainfrom
Conversation
Reviewer's GuideMigrates package metadata and optional dependencies from setup.py into pyproject.toml to streamline configuration and enable extras in editable installs. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @ChromatinRemodeling - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| long_description_content_type="text/markdown", | ||
| author='SAIUnit Developers', | ||
| author_email='chao.brain@qq.com', | ||
| packages=packages, |
There was a problem hiding this comment.
issue (bug_risk): Specify which packages to include in setup()
Without specifying packages, setuptools cannot determine which modules to include. Please add this back using find_packages() or specify packages in pyproject.toml.
There was a problem hiding this comment.
I believe the packages are already specified in the pyproject.toml.
There was a problem hiding this comment.
You’re right—if you’ve moved your find_packages() call into pyproject.toml under the setuptools section, then you don’t need to re-introduce it in setup.py. Just double-check that you have something like:
[tool.setuptools.packages.find]
include = ["saiunit*"]
exclude = ["docs*", "build*", "dev*", "dist*", "saiunit.egg-info*", "saiunit/__pycache__*"]
in your pyproject.toml. As long as that’s present, setuptools will pick up your packages automatically and you can safely leave setup.py lean. If you’re not seeing your modules packaged, then add that snippet (or the equivalent where/include settings) back into the TOML.
| testing = [ | ||
| 'pytest', | ||
| ] | ||
| cpu = ["jax[cpu]"] |
There was a problem hiding this comment.
issue: Extras need to be under [project.optional-dependencies]
Place cpu, cuda12, and tpu under [project.optional-dependencies] to ensure pip recognizes them as installable extras.
There was a problem hiding this comment.
The extras are indeed under [project.optional-dependencies].
There was a problem hiding this comment.
You’re right—those lines are already in the [project.optional-dependencies] table. My bad, that comment was misplaced. I’ve double-checked the TOML and the extras (cpu, cuda12, tpu) are indeed nested correctly under:
[project.optional-dependencies]
testing = ["pytest"]
cpu = ["jax[cpu]"]
…
I’ll resolve that thread. Thanks for catching it! If you haven’t already, it might be worth doing a quick smoke-test:
pip install -e '.[cpu]'
to make sure pip picks up the extra exactly as intended. Otherwise, looks good to me.
|
How to build wheel using |
IIUC it's as simple as According to https://packaging.python.org/en/latest/guides/writing-pyproject-toml/:
|
|
Thanks. Please update command lines in |
…obrain#36) * Refactor debug_info import and update version check in wrap_init * Bump version to 0.0.14
|
I updated the command lines and also made some changes to avoid warnings. Some notable changes are:
Solution: updated the project.license as a simple SPDX string Also: Solution: removed the license classifier line.
Solution: Remove |
|
I am unable to test the |
Description
I do this to add the ability to specify extras in editable installs. Example:
pip install -e ./saiunit[cuda12].How Has This Been Tested
Passed CI.
Types of changes
Checklist
Summary by Sourcery
Move project metadata (name, description, author, license, classifiers, keywords, dependencies, and extras) from setup.py into pyproject.toml to support specifying extras in editable installs
Enhancements: