-
Notifications
You must be signed in to change notification settings - Fork 92
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
disable mac installs for now in setup.py #472
disable mac installs for now in setup.py #472
Conversation
Hello @Moritz-Alexander-Kern! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2022-04-14 14:32:27 UTC |
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.
Better than removing for mac builds, I suggest to only add the fim module for Windows
and Linux
platforms.
Also, in the if-then-else clause on line 43, I suggest to specifically list elif platform.system()=="Linux":
setup.py
Outdated
"version": version, | ||
"packages": ['elephant', 'elephant.test'], | ||
"include_package_data": True, | ||
"ext_modules": [fim_module], |
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.
Remove here...
setup.py
Outdated
) | ||
} | ||
# do not compile external modules on darwin | ||
if platform.system() == "Darwin": |
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.
...and add the extension module here for Windows
and Linux
platforms.
This PR addresses Issue #469 .
Thanks @heplesser for reporting this.
Bug:
Pip-installing Elephant fails with a compiler error under macOS 12.3.
Fix:
Disable mac installs for now in
setup.py
.