Introduce OPENSIM_PYTHON_CONDA #2989
Conversation
…opensim-core into support_conda_build
try to force numpy install using python3
Use pip instead of pip3
There was a problem hiding this comment.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion
OpenSim/Simulation/Model/Frame.h, line 325 at r1 (raw file):
/** Accessor for Rotation matrix of the Frame in Ground. */ SimTK::Rotation getRotationInGround(const SimTK::State& state) const {
@carmichaelong Here's the template expansion that was missed by SWIG
…opensim-core into support_conda_build
Use more recent version of setup-python on windows to try force using python 3.8 throughout
Try passing Python3_ROOT_DIR on command line to CMake
Fix formatting
…opensim-core into support_conda_build
|
This PR practically moves us to a more recent CMake so that we can use Python3 constructs and NOT specify numpy externally which has been a nightmare for us and doesn't play well with conda-build. |
|
@carmichaelong or @nickbianco for quick review |
There was a problem hiding this comment.
Looks pretty straight-forward. I just had one quick question about how NumPy is handled now.
Reviewed 1 of 4 files at r1, 2 of 3 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @aymanhab and @carmichaelong)
CMakeLists.txt, line 552 at r4 (raw file):
if(${BUILD_PYTHON_WRAPPING}) set(required_python_version 3) find_package(Python3 3.6 REQUIRED COMPONENTS Interpreter Development NumPy)
Not blocking, just curious: how does including NumPy as a required component change building the bindings? It seems like you still need to provide the NumPy include dirs, it would be nice if it found them automatically.
There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @carmichaelong and @nickbianco)
CMakeLists.txt, line 552 at r4 (raw file):
Previously, nickbianco (Nicholas Bianco) wrote…
Not blocking, just curious: how does including NumPy as a required component change building the bindings? It seems like you still need to provide the NumPy include dirs, it would be nice if it found them automatically.
You do not need to specify Numpy directory on your own, instead it is auto-populated by CMake find mechanism which guarantees that the Numpy is found within the same Python3 installation (as echoed by the messages I added below)
There was a problem hiding this comment.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @aymanhab and @carmichaelong)
CMakeLists.txt, line 552 at r4 (raw file):
Previously, aymanhab (Ayman Habib) wrote…
You do not need to specify Numpy directory on your own, instead it is auto-populated by CMake find mechanism which guarantees that the Numpy is found within the same Python3 installation (as echoed by the messages I added below)
That's great! Thanks for clarifying.
|
Awesome, thanks for the review @nickbianco 👍 |
Fixes issue #0
Brief summary of changes
Testing I've completed
Looking for feedback on...
CHANGELOG.md (choose one)
This change is