Simplify extension setup. #11964
Merged
Simplify extension setup. #11964
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
anntzer
commented
Sep 1, 2018
anntzer
force-pushed
the
delayedextension
branch
from
a93799e to
d8258ed
Compare
Contributor
Author
|
Rebased on top of #11983 which makes things a bit simpler. |
anntzer
force-pushed
the
delayedextension
branch
from
d8258ed to
82de4f1
Compare
6 tasks
6 tasks
Member
anntzer
force-pushed
the
delayedextension
branch
from
82de4f1 to
aeb03c6
Compare
Contributor
Author
|
Done. |
jklymak
added
the
Release critical
Member
|
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. |
Member
|
Sorry, can’t help either. Not experienced in setuptools. |
tacaswell
approved these changes
Feb 25, 2019
Member
tacaswell
left a comment
tacaswell
left a comment
There was a problem hiding this comment.
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.
anntzer
force-pushed
the
delayedextension
branch
from
aeb03c6 to
ecf5f00
Compare
Contributor
Author
|
@tacaswell do you want to entice a second reviewer? :) |
QuLogic
approved these changes
Feb 27, 2019
Member
QuLogic
left a comment
QuLogic
left a comment
There was a problem hiding this comment.
Dunno if you want to change this one line.
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.
anntzer
force-pushed
the
delayedextension
branch
from
ecf5f00 to
6827449
Compare
Member
|
Going to merge based on two previous +ive reviews. |
meeseeksmachine
pushed a commit
to meeseeksmachine/matplotlib
that referenced
this pull request
Feb 27, 2019
dstansby
added a commit
that referenced
this pull request
Feb 27, 2019
…964-on-v3.1.x Backport PR #11964 on branch v3.1.x (Simplify extension setup.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Also closes #6928 by not attempting to reload numpy anymore.
PR Summary
PR Checklist