-
Notifications
You must be signed in to change notification settings - Fork 19.5k
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
Move project metadata from setup.py to pyproject.toml #20427
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #20427 +/- ##
==========================================
+ Coverage 77.41% 81.99% +4.58%
==========================================
Files 514 514
Lines 47118 47180 +62
Branches 7400 7409 +9
==========================================
+ Hits 36478 38687 +2209
+ Misses 8916 6704 -2212
- Partials 1724 1789 +65
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Thanks for the PR!
dependencies = {file = "requirements-common.txt"} | ||
|
||
[tool.setuptools.packages.find] | ||
include = ["keras", "keras.*"] |
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.
No excludes?
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.
setup.py had exclude=("*_test.py", "benchmarks")
, but this didn't do anything, so was dropped with the transition. The reason why it didn't do anything is that only Python modules under "keras" are found (thus "benchmarks" was already excluded), and individual Python files cannot be excluded -- only modules can be excluded. But I see that pip_build.py has logic to ignore the "_test.py" files.
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.
LGTM, thank you!
* Move project metadata from setup.py to pyproject.toml * Override black target version (for now) to avoid other changes * PR feedback * Move explicit list of dependencies from setup.py to pyproject.toml * pathlib was already imported
This PR take's on PEP 621 as supported by setuptools to move the project's metadata from
setup.py
topyproject.toml
.Dynamic features that are supported fetch the version from
keras.src.version.__version__
and the common dependencies from.requirements-common.txt
(it cannot readrequirements.txt
, as only a subset of the requirements format is supported)Modern projects shouldn't need
setup.py
anymore, so this file is removed. But if it is expected, a shim withfrom setuptools import setup; setup()
can be added.pip_build.py
is updated to support the changes, as well as a few GHA workflows.