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

Setuptools related improvements #1492

Merged
merged 5 commits into from
Nov 9, 2023

Conversation

sanjayankur31
Copy link
Contributor

I'm going to preface this PR by saying that I do not know why the current upstream configuration works. From what I see in the various docs, it shouldn't, but there's so much going on in the Python packaging ecosystem that I'm no longer confident I understand it all properly 😂

While building on Fedora, the current pyproject.toml etc., doesn't work---it only includes the top level brian2 package in the generated wheels. This seems to be the correct behaviour because as the setuptools documentation notes (or suggests rather), only including the top level package name in the package list is not enough. One either has to list all sub-packages, or use the discovery tools.

https://setuptools.pypa.io/en/latest/userguide/package_discovery.html#package-discovery-and-namespace-packages

So, to ensure that all sub-packages and the necessary template/data is included, I had to tweak these files as noted in the PR. (the tests from my local build are still running, so there may be more data files that are needed to be added to the manifest etc.).

@mstimberg
Copy link
Member

Hi @sanjayankur31 thanks for your PR! I have to say I am now more confused than ever – if I remember correctly, I only arrived at the current solution by trial and error, as you say, the documentation seems to suggest it shouldn't work. But then, our cibuildwheel-based wheels do contain all the subpackages (and templates, etc.), and the same for the source package we build with python -m build. Weird.
But of course everything still works with your additions, and if it fixes building on Fedora, I'm of course happy to merge it.

@mstimberg mstimberg merged commit 7e0b610 into brian-team:master Nov 9, 2023
25 checks passed
@sanjayankur31 sanjayankur31 deleted the fix/setuptools branch November 9, 2023 14:25
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.

2 participants