Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Simplify extension setup.#11964
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Rebased on top of#11983 which makes things a bit simpler. |
Done. |
Marking release critical so it gets some love.. I'd review, but really don't grok whats going on here. But if someone more knowledgeable reviewed, I'd be comfortable approving as not doing anything dangerous. |
Sorry, can’t help either. Not experienced in setuptools. |
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.
As discussed on call this simplifies the build process and we will not discover any edge cases until we try to build all of the wheels / packages.
@tacaswell do you want to entice a second reviewer? :) |
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.
Dunno if you want to change this one line.
Uh oh!
There was an error while loading.Please reload this page.
setupext currently has a complex machinery to define extension modules.Essentially, the problem is that numpy may not be installed at the timethe extensions are declared, so we can't call np.get_include() to getthe include paths. So we use a DelayedExtension class and a hookmechanism to inject the numpy include path later, once it becomesavailable.Instead, we can just declare a dummy extension (so that setuptoolsdoesn't elide the call to build_ext), then swap in the correctextensions in build_ext.finalize_options(): at that point, numpy willhave been installed and we don't need any crazy machinery.
Going to merge based on two previous +ive reviews. |
…964-on-v3.1.xBackport PR#11964 on branch v3.1.x (Simplify extension setup.)
Uh oh!
There was an error while loading.Please reload this page.
setupext currently has a complex machinery to define extension modules.
Essentially, the problem is that numpy may not be installed at the time
the extensions are declared, so we can't call np.get_include() to get
the include paths. So we use a DelayedExtension class and a hook
mechanism to inject the numpy include path later, once it becomes
available.
Instead, we can just declare a dummy extension (so that setuptools
doesn't elide the call to build_ext), then swap in the correct
extensions in build_ext.finalize_options(): at that point, numpy will
have been installed and we don't need any crazy machinery.
Alsocloses#6928 by not attempting to reload numpy anymore.
PR Summary
PR Checklist