Skip to content
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

Implement numpy hack in setup.py to enable install under Poetry #3363

Merged
merged 2 commits into from
Dec 3, 2022

Conversation

jaymegordo
Copy link
Contributor

@jaymegordo jaymegordo commented Jul 8, 2022

Fixes #3362

Confirmed poetry install works on M1 mbp with this fix.

poetry add git+ssh://git@github.com:jaymegordo/gensim.git#fix_3362

@piskvorky
Copy link
Owner

Thanks! Let's wait for @mpenkov 's review and we can merge.

@piskvorky piskvorky requested a review from mpenkov July 8, 2022 19:23
@piskvorky piskvorky added this to the Next release milestone Jul 8, 2022
@jaymegordo
Copy link
Contributor Author

Btw I literally just copied the first fix I googled for haha, no idea if there's a more elegant way to do it, or really what any of the __builtins__ stuff does.

@jaymegordo
Copy link
Contributor Author

Also will resolve #3225

@vladyslav-burylov
Copy link

Hi Team,

We also facing an issue with this: without this fix, gensim cannot be installed via poetry 1.2.
Any chance this can be reviewed and merged?

Appreciating your help in advance.

@mpenkov mpenkov changed the title Closes #3362: Install issue poetry Implement numpy hack in setup.py to enable install under Poetry Aug 21, 2022
setup.py Outdated
@@ -104,7 +104,15 @@ def finalize_options(self):
build_ext.finalize_options(self)
# Prevent numpy from thinking it is still in its setup process:
# https://docs.python.org/2/library/__builtin__.html#module-__builtin__
__builtins__.__NUMPY_SETUP__ = False
try:
__builtins__.__NUMPY_SETUP__ = False
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular part appears to be for Python 2 - we don't need it, right?

setup.py Outdated
import builtins
builtins.__NUMPY_SETUP__ = False
except:
print("Skipping numpy hack; if installation fails, try installing numpy first")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exception can possibly get raised here?

Assuming that some exception can, then a explicit error message would be better:

Suggested change
print("Skipping numpy hack; if installation fails, try installing numpy first")
print("Skipping numpy hack; if gensim installation fails, try installing numpy first")

Also, may as well output the reason why the hack failed (what exception was caught) and a trace.

simonseed added a commit to leerix/gensim that referenced this pull request Sep 5, 2022
Implement numpy hack in setup.py to enable install under Poetry piskvorky#3363
this code can never raise an exception, so we shouldn't be expecting
them
@mpenkov mpenkov merged commit 3331b82 into piskvorky:develop Dec 3, 2022
anubhavi25 pushed a commit to anubhavi25/gensim that referenced this pull request Nov 9, 2023
anubhavi25 pushed a commit to anubhavi25/gensim that referenced this pull request Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Install issue when installing with poetry
4 participants