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

Add py39 wheels to travis/azure #3058

Merged
merged 10 commits into from
Mar 15, 2021
5 changes: 4 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ matrix:
- os: linux
env:
- MB_PYTHON_VERSION=3.8
- os: linux
env:
- MB_PYTHON_VERSION=3.9
before_install:
- source multibuild/common_utils.sh
- source multibuild/travis_steps.sh
Expand All @@ -46,4 +49,4 @@ notifications:
email:
- penkov+gensimwheels@pm.me
on_success: always
on_failure: always
on_failure: always
3 changes: 3 additions & 0 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ strategy:
py38:
python.version: '3.8'
TOXENV: "py38-win"
py39:
python.version: '3.9'
TOXENV: "py39-win"

steps:
- task: UsePythonVersion@0
Expand Down
6 changes: 4 additions & 2 deletions gensim/similarities/levenshtein.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,10 @@ def levdist(t1, t2, max_distance=float("inf")):
The Levenshtein distance between `t1` and `t2`.

"""
import Levenshtein

try:
import Levenshtein
except ImportError:
raise ImportError("Levenshtein not installed. Please run `pip install Levenshtein`.")
Copy link
Collaborator

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.

Copy link
Contributor Author

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

Copy link
Collaborator

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.

distance = Levenshtein.distance(t1, t2)
if distance > max_distance:
return max(len(t1), len(t2))
Expand Down
21 changes: 20 additions & 1 deletion gensim/test/test_similarities.py
Original file line number Diff line number Diff line change
Expand Up @@ -1546,6 +1546,10 @@ def test_inner_product_corpus_corpus_true_true(self):

class TestLevenshteinDistance(unittest.TestCase):
def test_max_distance(self):
try:
Copy link
Collaborator

Choose a reason for hiding this comment

The 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 gensim/similarities/__init__.py:

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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))
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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))
Expand Down
20 changes: 12 additions & 8 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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 = [
Copy link
Collaborator

Choose a reason for hiding this comment

The 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',
    ])

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.
Expand All @@ -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',
Expand All @@ -308,7 +313,6 @@ def run(self):
'nltk',
'testfixtures',
'statsmodels',
'pyemd',
'pandas',
]

Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tox]
minversion = 2.0
envlist = {py36,py37,py38}-{win,linux}, flake8, docs, docs-upload, download-wheels, upload-wheels, test-pypi
envlist = {py36,py37,py38, py39}-{win,linux}, flake8, docs, docs-upload, download-wheels, upload-wheels, test-pypi
skipsdist = True
platform = linux: linux
win: win64
Expand Down