-
Notifications
You must be signed in to change notification settings - Fork 47
Upgrade python version #49
base: main
Are you sure you want to change the base?
Conversation
…erent python version (py3.9 primarily)
Hi @pperanich! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Thank you for your contribution on python version upgrade! Please also help us review the Facebook CLA, CLA is common for all Facebook public repository contributions. |
As Python Software Foundation has stopped the support for python3.6 on Dec 2021, and python3.8 is the recommended version (as the latest version with security Maintenance status from Python Foundation) for LabGraph, you can alternatively consider supporting python3.8+ in this Pull Request if there is no quick fix to get cibuildwheel tests passed. |
I'll look into that. The python versioning isn't the issue for the Linux cibuilds; I still expect the |
@jfResearchEng could you provide instructions on how the PyPi wheels were built for Windows and Mac? What were the steps to setup the build environment? That could help solve the issues in the cibuildwheel action. |
Instructions on Windows can be found here. Mac is similar with updates for directory in third-party/python and pybinding. |
Description
This PR aims to address the following:
Fixes #(issue)
#34
Type of change
Feature/Issue validation/testing
Dockerfile updates
To support compiling for python version after 3.6, an additional
-fPIC
position independent code flag was required in compiling python for themanylinux2014
image. A pull request is currently open (pypa/manylinux#1258) to merge this flag change into the manylinux repo. For testing, themanylinux2014
was compiled on my local machine, followed by adocker build .
, which would then use the localmanylinux2014
image. The defaultDockerfile
now compiles amanylinux
compatible wheel for python 3.6-3.10.cibuildwheel progress
A cibuildwheel action has been created to build Labgraph wheels for Mac, Windows, and Linux
DYLD_LIBRARY_PATH
(https://cibuildwheel.readthedocs.io/en/stable/faq/#macos-passing-dyld_library_path-to-delocate).Dockerfile.Windows
which is able to successfully compile labgraph in a container, and it sets the environment variables with the following:RUN C:\Program^ Files^ ^(x86^)\Microsoft^ Visual^ Studio\2019\BuildTools\VC\Auxiliary\Build\vcvars64.bat && python setup_py36.py build
Though I call
vcvars64.bat
inscripts/prepare_build_for_windows.bat
, the build job fails to set the environment correctly. You may be able to make use ofCIBW_ENVIRONMENT
(https://cibuildwheel.readthedocs.io/en/stable/options/#environment) to inject the correct variables into the build environment, though I have failed to find a way to execute the command and set the environment variables yet.