-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add default values for the packages
and py_modules
options
#2888
Conversation
The need for explicitly setting config options for `packages`, `py_modules` and `package_dir` is usually mentioned as a pitfall of setuptools and can be confusing some times. The changes introduced here explore the conventions of the Python ecosystem (namely ``flat-layout`` and ``src-layout`` for packages) to provide default values for these configurations. Notice that the default values will only be applied if the `py_modules` or `packages` options are not given. This is intentional, since the implementation is supposed to be backwards compatible. References: https://blog.ionelmc.ro/2015/02/24/the-problem-with-packaging-in-python/ https://blog.ionelmc.ro/2014/06/25/python-packaging-pitfalls/
return set(zipfile.namelist()) | ||
|
||
|
||
def _run_build(path): |
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.
In the case PR #2863 gets merged, this function can be refactored in terms of the run
function defined there.
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.
That did get merged, it seems. :)
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.
Hi @henryiii, thanks for the comment. I had forgotten about this one.
I ended up refactoring the code in #2894...
These 2 PRs have the same objective but different approaches.
Since #2894 goes in the direction pointed out in #2887, I think it has more chances of being merged in the future (I would like to have one of the two PRs merged once we add support for pyproject.toml metadata).
The improvements in #2894 can be easily ported over to this PR, if we think it is more suitable.
return {f for f in relative_files if f} | ||
|
||
|
||
def _get_wheel_members(wheel_path): |
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.
These 2 functions could be added to the Archive
class defined in the test helper for PR #2863.
Closed in favour of #2894 |
Motivation
The need for explicitly specifying
packages
andpy_modules
(together withpackage_dir
) is often criticized as a pitfall of setuptools.This is also a common cause of confusion (sometimes even frustration) between new users.
Since the majority of the packages in the ecosystem use the "src-layout", "flat-layout" or a "single-module" package, these options can be safely guessed by inspecting the directory structure and comparing with the available metadata (the
name
of the distribution).Summary of changes
This PR adds a few methods to the
dist
class, with the purpose of comparing the existing files and directories to the distribution name and the package layout conventions in order to decide which are the best values that could be set topy_modules
andpackages
.Please note that this change is meant to be backward compatible, and no attempt to fill-in values for the options will be done if one of them is set.
Empty distributions are still possible, as long as the package author don't create files and directories that coincide with the 3 layouts previously mentioned.
I believe this is a fair assumption, since the chances of an author that purposefully wants to publish an empty package (e.g. metapackage) including a
src
folder or a script/folder with the same name as the distribution are negligible.Closes #2887
Pull Request Checklist
changelog.d/
.(See documentation for details)