-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Add py39 wheels to travis/azure #3058
Changes from 5 commits
c83394c
8a5a103
516ead4
02bb45d
da33758
0b13177
c94991b
ba821d4
51d8410
ccc35ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1546,6 +1546,10 @@ def test_inner_product_corpus_corpus_true_true(self): | |
|
||
class TestLevenshteinDistance(unittest.TestCase): | ||
def test_max_distance(self): | ||
try: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit messy. You shouldn't be testing for dependencies in so many separate places (once in the actual source code, and multiple times in tests). I think there's a better way to do it. First, in import warning
try:
import Levenshtein
except ImportError:
warning.warn(
"The gensim.similarities.levenshtein submodule is disabled, because the optional "
"Levenshtein package <https://pypi.org/project/python-Levenshtein/> is unavailable. "
"Install Levenhstein (e.g. `pip install Levenshtein`) to suppress this warning."
)
LevenshteinSimilarityIndex = None
else:
from .levenshtein import LevenshteinSimilarityIndex Then in your tests: import gensim.similarities
@unittest.skipIf(gensim.similarities.LevenshteinSimilarityIndex is None, "gensim.similarities.levenshtein is disabled")
def test_method(...):
pass There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, this reads far better |
||
import Levenshtein # noqa:F401 | ||
except ImportError as e: | ||
raise unittest.SkipTest("Levenshtein library is not available: %s" % e) | ||
t1 = "holiday" | ||
t2 = "day" | ||
max_distance = max(len(t1), len(t2)) | ||
|
@@ -1558,12 +1562,20 @@ def test_max_distance(self): | |
|
||
class TestLevenshteinSimilarity(unittest.TestCase): | ||
def test_empty_strings(self): | ||
try: | ||
import Levenshtein # noqa:F401 | ||
except ImportError as e: | ||
raise unittest.SkipTest("Levenshtein library is not available: %s" % e) | ||
t1 = "" | ||
t2 = "" | ||
|
||
self.assertEqual(1.0, levsim(t1, t2)) | ||
|
||
def test_negative_hyperparameters(self): | ||
try: | ||
import Levenshtein # noqa:F401 | ||
except ImportError as e: | ||
raise unittest.SkipTest("Levenshtein library is not available: %s" % e) | ||
t1 = "holiday" | ||
t2 = "day" | ||
alpha = 2.0 | ||
|
@@ -1579,6 +1591,10 @@ def test_negative_hyperparameters(self): | |
levsim(t1, t2, -alpha, -beta) | ||
|
||
def test_min_similarity(self): | ||
try: | ||
import Levenshtein # noqa:F401 | ||
except ImportError as e: | ||
raise unittest.SkipTest("Levenshtein library is not available: %s" % e) | ||
t1 = "holiday" | ||
t2 = "day" | ||
alpha = 2.0 | ||
|
@@ -1609,7 +1625,10 @@ def setUp(self): | |
|
||
def test_most_similar(self): | ||
"""Test most_similar returns expected results.""" | ||
|
||
try: | ||
import Levenshtein # noqa:F401 | ||
except ImportError as e: | ||
raise unittest.SkipTest("Levenshtein library is not available: %s" % e) | ||
index = LevenshteinSimilarityIndex(self.dictionary) | ||
results = list(index.most_similar(u"holiday", topn=1)) | ||
self.assertLess(0, len(results)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,9 +17,9 @@ | |
import platform | ||
import shutil | ||
import sys | ||
from setuptools import setup, find_packages, Extension | ||
from setuptools.command.build_ext import build_ext | ||
|
||
from setuptools import Extension, find_packages, setup | ||
from setuptools.command.build_ext import build_ext | ||
|
||
c_extensions = { | ||
'gensim.models.word2vec_inner': 'gensim/models/word2vec_inner.c', | ||
|
@@ -270,20 +270,25 @@ def run(self): | |
# 'pytest-rerunfailures', # disabled 2020-08-28 for <https://github.com/pytest-dev/pytest-rerunfailures/issues/128> | ||
'mock', | ||
'cython', | ||
'nmslib', | ||
'pyemd', | ||
'testfixtures', | ||
'Morfessor==2.0.2a4', | ||
] | ||
|
||
not_py39_win_testenv = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is slightly more complicated than it really needs to be. You want to avoid installing these packages for Python 3.9 under Windows, and install them everywhere else. So why not do something like: if not (sys.platform.lower().startswith("win") and sys.version[:2] >= (3, 9)):
core_testenv.extend([
'pyemd',
'nmslib',
'python-Levenshtein >= 0.10.2',
]) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that also makes far more sense. I'll make these changes tomorrow |
||
'pyemd', | ||
'nmslib', | ||
'python-Levenshtein >= 0.10.2', | ||
] | ||
|
||
# Add additional requirements for testing on Linux that are skipped on Windows. | ||
linux_testenv = core_testenv[:] + visdom_req + ['pyemd', ] | ||
linux_testenv = core_testenv[:] + visdom_req + not_py39_win_testenv | ||
|
||
# Skip problematic/uninstallable packages (& thus related conditional tests) in Windows builds. | ||
# We still test them in Linux via Travis, see linux_testenv above. | ||
# See https://github.com/RaRe-Technologies/gensim/pull/2814 | ||
win_testenv = core_testenv[:] | ||
win_testenv = core_testenv[:] + not_py39_win_testenv | ||
if sys.version_info > (3,8,999): # py 3.8.1 is greater than 3.8 so set micro to 999 | ||
win_testenv = core_testenv[:] | ||
|
||
# | ||
# This list partially duplicates requirements_docs.txt. | ||
|
@@ -296,7 +301,7 @@ def run(self): | |
# https://packaging.python.org/discussions/install-requires-vs-requirements/ | ||
# | ||
|
||
docs_testenv = core_testenv + distributed_env + visdom_req + [ | ||
docs_testenv = core_testenv + not_py39_win_testenv + distributed_env + visdom_req + [ | ||
'sphinx <= 2.4.4', # avoid `sphinx >= 3.0` that breaks the build | ||
'sphinx-gallery', | ||
'sphinxcontrib.programoutput', | ||
|
@@ -308,7 +313,6 @@ def run(self): | |
'nltk', | ||
'testfixtures', | ||
'statsmodels', | ||
'pyemd', | ||
'pandas', | ||
] | ||
|
||
|
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.
Careful, the PyPI name of the library we need is https://pypi.org/project/python-Levenshtein/, not
Levenshtein
.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.
Thanks, my bad there! Slight oversight. Will fix
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.
BTW, I don't think we need this try-except here if we catch the import error in gensim.similarities.init.