Refactor setup.py #229
Refactor setup.py #229
Conversation
|
Hey @brownj85, thanks for this! the new I just want to check if this is ready for review, as I can see that the linux build is failing. |
Hey @josh146, I noticed that - but it doesn't seem to be related to this PR. I re-ran it on master, which passed previously, and it failed: https://app.circleci.com/pipelines/github/XanaduAI/thewalrus/1034/workflows/ad44f38f-423c-4bcb-b979-c87cb42d05cd/jobs/3188. |
Codecov Report
@@ Coverage Diff @@
## master #229 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 20 21 +1
Lines 1187 1200 +13
=========================================
+ Hits 1187 1200 +13
Continue to review full report at Codecov.
|
Weird 🤔 Was this fixed by bumping the CircleCI docker image to 3.6.12? |
| config["include_dirs"].append(EIGEN_INCLUDE_DIR) | ||
| else: | ||
| config["include_dirs"].extend( | ||
| ("/usr/include/eigen3", "/usr/local/include/eigen3") |
There was a problem hiding this comment.
We could define above
EIGEN_INCLUDE_DIR = ("/usr/include/eigen3", "/usr/local/include/eigen3", os.environ.get("EIGEN_INCLUDE_DIR")to avoid the if statement here?
There was a problem hiding this comment.
No, the issue with this is that if Eigen is installed in the system and the user provides a custom path, there's no way to know which one the compiler will pick. The if statement ensures that the user-provided Eigen headers will always be used
Context:
If the user sets the
CFLAGSenvironment variable, the build fails.Description of the Change:
User-provided
CFLAGSwill not override the flags necessary for compilation (in particular-fopenmp). General improvements to the readability of the build script. Will no longer attempt to build without Cython, since it's not possible to do so (libwalrus.cppdoes not exist).Benefits:
Fixes bug, nicer build script.
Possible Drawbacks:
None
Related GitHub Issues:
Fixes #198.