-
Notifications
You must be signed in to change notification settings - Fork 16
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
Refactor collect_package_data to make logic clearer #120
Conversation
This is a preliminary refactoring for fixing python/typeshed#11254. The idea is that the new class `PackageData` encapsulates all data concerning packages and their contents. This allows us later to find the top-level non-namespace packages, instead of just the top-level packages as we are doing now.
elif entry.is_file() and entry.suffix == ".pyi": | ||
pkg_name = entry.stem | ||
# Module -> package transformation is done while copying. | ||
package_data[pkg_name] = ["__init__.pyi"] | ||
stub_files = ["__init__.pyi"] | ||
elif entry.is_dir(): | ||
pkg_name = entry.name | ||
stub_files = find_stub_files(str(entry)) | ||
else: | ||
if entry.name == TESTS_NAMESPACE: | ||
continue | ||
pkg_name = entry.name + SUFFIX | ||
package_data[pkg_name] = find_stub_files(str(entry)) | ||
package_data[pkg_name].append(META) | ||
raise ValueError(f"Only stub files are allowed, not {entry.name!r}") | ||
package_data[pkg_name + SUFFIX] = [*stub_files, META] |
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.
Maybe you won't wanna do that now, but since you're touching this area of code I'll mention it:
With #112 (setuptools v69) , the stub_files
logic could become unnecessary.
According to https://setuptools.pypa.io/en/stable/userguide/miscellaneous.html#controlling-files-in-the-distribution this is an opt-out feature, so you shouldn't need to add anything new either.
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.
Let's punt this to a separate PR to keep this as a simple refactoring.
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.
New logic checks out and is easier to reason about.
Depends on #119