-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
BLD Remove build_utils/
from the built wheels, and associated Meson cleanups
#29821
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
BLD Remove build_utils/
from the built wheels, and associated Meson cleanups
#29821
Conversation
#!/usr/bin/env python3 | ||
|
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.
For the reason why this shebang has been added, please refer to numpy/numpy#24971.
The wheels build fine, but the |
I pushed a commit that I think fixes it c5a71bf |
Oops, yes – thanks! |
Oh well not so simple, looks like this breaks the test discovery see build log ...
|
I think there should be a way to manoeuvre around it for the tests. I guess it's because of the |
The solution to it is from my perspective will be to use PRs - scipy/scipy#20712 |
Thanks for chiming in, @czgdp1807. I've experimented with adding install tags locally, and doing that seems to be a larger change which would need more consideration and reviewing effort in another PR (it's certainly useful to do, though); is there a more—for the lack of a better word—"organic" method to exclude files from sdists (and subsequently, wheels) in the meantime? In any case, I plan to revisit this PR next week. |
Closing this PR as #31058 is now merged. Thank you! |
Reference Issues/PRs
Stemmed off from #29791
What does this implement/fix? Explain your changes.
This PR aims to remove the
sklearn/_build_utils/
folder from the wheel (while keeping it in the sdist to allow building wheels). The utility filesklearn/_build_utils/tempita.py
seems to have created a runtime dependency on Cython in the PR linked above that aims to improve the Pyodide builds, because Cython is being imported at the time of running the test suite. I'm unsure how it works for the current builds with the[pyodide]
commit marker, but either way: since the_build_utils/
files are needed at just build time, it would make sense to remove them from the wheel artifacts.This behaviour matches that of NumPy, where the contents of this folder have likely been inspired from back in #28040. The NumPy source distribution on PyPI includes this folder since it's needed at build-time to build wheels off of the sdist, but removes it from the wheel. There are a few more potential and possible Meson-related cleanups to do here but I am not too eager to push them in this PR to keep the diff small :)
In particular, here's a short summary of the changes:
files()
with the more standardfind_program()
exclude_directories:
keyword argument forinstall_subdir()
to remove the build utilities from the wheels from the installationtempita.py
CLI utility to invoke thetempita
executable directlyAny other comments?
N/A