Skip to content
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

fix: Use include rather than exclude to find_namespace_packages in setup.py #502

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

smoser
Copy link
Contributor

@smoser smoser commented Nov 1, 2024

https://setuptools.pypa.io/en/latest/userguide/package_discovery.html has a warning:

Please have in mind that find_namespace: (setup.cfg),
find_namespace_packages() (setup.py) and find (pyproject.toml)
will scan all folders that you have in your project directory
if you use a flat-layout.

That applies here. Without this change, if you run: 'python3 setup.py bdist_wheel' then you may end up with build/ in your wheel, and in some cases even
docs/ and testing/ .

The fix is to only include proto and proto.*.

Fixes #503

@smoser smoser requested a review from a team as a code owner November 1, 2024 18:45
https://setuptools.pypa.io/en/latest/userguide/package_discovery.html
has a warning:

> Please have in mind that find_namespace: (setup.cfg),
> find_namespace_packages() (setup.py) and find (pyproject.toml)
> will scan all folders that you have in your project directory
> if you use a flat-layout.

That applies here.  Without this change, if you run:
'python3 setup.py bdist_wheel' then you may end up with
build/ in your wheel, and in some cases even
docs/ and testing/ .

The fix is to only include proto and proto.*.

Signed-off-by: Scott Moser <[email protected]>
@smoser
Copy link
Contributor Author

smoser commented Nov 6, 2024

After a lot more testing and debugging I have a better idea of what is going on.
the presence of setuptools-scm in the wolfi build environment was what was ultimately causing 'docs/' and 'testing/' to get pulled into the wheel file.

I now think this is right, and is in-line with what is happening in other googleapis projects like https://github.com/googleapis/python-aiplatform/blob/f1da73b10d94d55a76d29de23fc3e198a37f78b6/setup.py#L36 and https://github.com/googleapis/python-pubsub/blob/5c78ac961199cd094ac4d29634fa4b6bfb73436a/setup.py#L62 . Its better to be explicit and have to extend this list if there is ever a need.

I can modify the PR to be more like one of those if requested.

@smoser smoser marked this pull request as ready for review November 6, 2024 03:44
@parthea parthea self-assigned this Jan 22, 2025
@parthea parthea changed the title Use include rather than exclude to find_namespace_packages in setup.py fix: Use include rather than exclude to find_namespace_packages in setup.py Jan 22, 2025
@parthea
Copy link
Contributor

parthea commented Jan 22, 2025

Thanks @smoser!

@parthea parthea enabled auto-merge (squash) January 22, 2025 15:26
@parthea parthea added the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 22, 2025
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Jan 22, 2025
@parthea parthea merged commit 77e252e into googleapis:main Jan 22, 2025
26 checks passed
parthea added a commit that referenced this pull request Jan 22, 2025
* migrate to pyproject.toml

* Rebase following #502

* add entries such as documentation and classifiers which exist in other projects

* Use correct url for docs

---------

Co-authored-by: Anthonios Partheniou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

running setup.py bdist_wheel results in build/, docs/ and testing/ in the wheel
2 participants