From 865d55b12b5daba18fadd919b31bc9793c900b41 Mon Sep 17 00:00:00 2001 From: Erik Forsberg Date: Tue, 20 Oct 2020 09:39:36 +0200 Subject: [PATCH 1/4] Fix mypy error for field_for_type function. Fixes #672 --- faust/models/fields.py | 9 +++++++-- faust/models/record.py | 5 ++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/faust/models/fields.py b/faust/models/fields.py index c57fa0be8..a3153ad53 100644 --- a/faust/models/fields.py +++ b/faust/models/fields.py @@ -7,6 +7,7 @@ from typing import ( Any, Callable, + Hashable, Iterable, Mapping, Optional, @@ -544,7 +545,10 @@ def prepare_value(self, value: Any, *, @lru_cache(maxsize=2048) def field_for_type( - typ: Type) -> Tuple[Type[FieldDescriptorT], Optional[Type[Tag]]]: + htyp: Hashable) -> Tuple[Type[FieldDescriptorT], Optional[Type[Tag]]]: + # This is a way to make mypy >= 0.790 happy, as lru_cache + # expects a Hashable + typ = cast(Type, htyp) try: # 1) Check if type is in fast index. return TYPE_TO_FIELD[typ], None @@ -557,7 +561,8 @@ def field_for_type( else: try: if origin is not None and issubclass(origin, Tag): - return field_for_type(typ.__args__[0])[0], typ + return field_for_type( + cast(Hashable, typ.__args__[0]))[0], typ except TypeError: pass diff --git a/faust/models/record.py b/faust/models/record.py index ca1d26bc9..dd5981f7c 100644 --- a/faust/models/record.py +++ b/faust/models/record.py @@ -7,6 +7,7 @@ Callable, Dict, FrozenSet, + Hashable, List, Mapping, MutableMapping, @@ -253,7 +254,9 @@ def add_related_to_tagged_indices(field: str, else: target_type = typ if descr is None or not isinstance(descr, FieldDescriptorT): - DescriptorType, tag = field_for_type(target_type) + # Make mypy happy + hashed_target_type = cast(Hashable, target_type) + DescriptorType, tag = field_for_type(hashed_target_type) if tag: add_to_tagged_indices(field, tag) descr = DescriptorType( From 15febb223e4581d290a62780506bd48e84c508bd Mon Sep 17 00:00:00 2001 From: Erik Forsberg Date: Wed, 21 Oct 2020 10:06:52 +0200 Subject: [PATCH 2/4] Make Cython testing actually work Previously, the cython stage in Travis would build (I think) but not use the .so files during testing, because of how py.test adds the current working directory to sys.path - the .so files would be installed in the tox virtualenv, but python would import faust from the faust/ directory under the git clone. This made tests that would otherwise fail with the Cython implementation pass, as they were testing the (pure) python implementation of modules. Also adds one test under t/cython that will fail if we can't import a cythonized module, ensuring we get a proper warning if we have an incorrect test environment. --- .travis.yml | 9 ++------- t/cython/__init__.py | 0 t/cython/test_cython.py | 18 ++++++++++++++++++ t/unit/windows/test_hopping_window.py | 1 + tox.ini | 13 +++++++++++-- 5 files changed, 32 insertions(+), 9 deletions(-) create mode 100644 t/cython/__init__.py create mode 100644 t/cython/test_cython.py diff --git a/.travis.yml b/.travis.yml index 46c2f33a5..c2c359052 100644 --- a/.travis.yml +++ b/.travis.yml @@ -143,7 +143,7 @@ matrix: - export PATH="/c/Python37:/c/Python37/Scripts:$PATH" - python -m pip install --upgrade pip wheel - python: 3.8.0 - env: TOXENV=3.8 IDENT= RUN_SUITE=y USE_CYTHON=y NO_CYTHON= + env: TOXENV=3.8-cython IDENT= RUN_SUITE=y USE_CYTHON=y NO_CYTHON= os: linux dist: xenial stage: cython @@ -152,12 +152,7 @@ before_install: - if [[ "$TRAVIS_OS_NAME" == "linux" && "$USE_ENCHANT" == "y" ]]; then sudo apt-get install -y enchant libenchant-dev; fi install: -- | - if [[ "$USE_CYTHON" == "y" ]]; then - python -m pip install -U cython tox - else - python -m pip install -U tox - fi +- python -m pip install -U tox after_success: - | if [[ ! -z "$IDENT" ]]; then diff --git a/t/cython/__init__.py b/t/cython/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/t/cython/test_cython.py b/t/cython/test_cython.py new file mode 100644 index 000000000..8e25945fc --- /dev/null +++ b/t/cython/test_cython.py @@ -0,0 +1,18 @@ +import os +import pytest + + +class test_Cython: + """Test that we can import Cython modules when run with Cython enabled + This test is here to ensure we get a clear indication if e.g. Travis + fails to correctly install Cython and build our modules + """ + @pytest.mark.skipif(not bool(os.environ.get('USE_CYTHON')), + reason='Cython not enabled') + @pytest.mark.skipif(bool(os.environ.get('NO_CYTHON', False)), + reason='Cython disabled') + def test_import(self): + from faust._cython.windows import HoppingWindow + HoppingWindow(60, 60) + # No assert, just check that we could import and init + # without exception diff --git a/t/unit/windows/test_hopping_window.py b/t/unit/windows/test_hopping_window.py index 48af66582..9e60e3d1a 100644 --- a/t/unit/windows/test_hopping_window.py +++ b/t/unit/windows/test_hopping_window.py @@ -1,3 +1,4 @@ +from pytest import approx from faust.windows import HoppingWindow diff --git a/tox.ini b/tox.ini index af19bb226..5412e3a68 100644 --- a/tox.ini +++ b/tox.ini @@ -11,13 +11,18 @@ deps= spell: -r{toxinidir}/requirements/spell.txt flake8,docstyle: -r{toxinidir}/requirements/dist.txt bandit: bandit + 3.8-cython: -r{toxinidir}/requirements/extras/cython.txt passenv = http_proxy HTTP_PROXY https_proxy HTTPS_PROXY no_proxy NO_PROXY sitepackages = False recreate = False -commands = py.test --random-order --open-files -xvv --cov=faust t/unit t/functional t/integration t/meticulous/ t/regression +# Use append import mode, or we will not be testing against the packages version of faust installed +# into the tox virtualenv, but rather against the faust/ directory from current working directory. +# This causes trouble when testing Cythonized modules, as the .so files will be installed +# in the virtualenv, but not under faust/ +commands = py.test --import-mode append --random-order --open-files -xvv --cov=faust t/unit t/functional t/integration t/meticulous/ t/regression basepython = - 3.8,flake8,apicheck,linkcheck,configcheck,typecheck,docstyle,bandit,spell: python3.8 + 3.8,flake8,apicheck,linkcheck,configcheck,typecheck,docstyle,bandit,spell,3.8-cython: python3.8 3.7: python3.7 3.6: python3.6 @@ -55,3 +60,7 @@ commands = [testenv:bandit] commands = bandit -b extra/bandit/baseline.json -c extra/bandit/config.yaml -r faust + +[testenv:3.8-cython] +commands = {[testenv]commands} t/cython +passenv = {[testenv]passenv} USE_CYTHON NO_CYTHON \ No newline at end of file From e3f8c022d77be83cc44c65958dad94ca53198b89 Mon Sep 17 00:00:00 2001 From: Erik Forsberg Date: Wed, 21 Oct 2020 12:55:08 +0200 Subject: [PATCH 3/4] Adds test for #674 that fails only on Cython --- t/unit/windows/test_hopping_window.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/t/unit/windows/test_hopping_window.py b/t/unit/windows/test_hopping_window.py index 9e60e3d1a..95a6c3c41 100644 --- a/t/unit/windows/test_hopping_window.py +++ b/t/unit/windows/test_hopping_window.py @@ -60,3 +60,19 @@ def test_stale_timestamp(self): for time in range(0, now_timestamp - expires): print(f'TIME: {time} NOW TIMESTAMP: {now_timestamp}') assert window.stale(time, now_timestamp) is True + + def test_ranges_types(self): + size = 60 + step = 60 + window = HoppingWindow(size, step) + + # There's nothing special about this timestamp, + # it was simply when the test was created + timestamp = 1603122451.544989 + window_ranges = window.ranges(timestamp) + + assert len(window_ranges) == 1 + assert type(window_ranges[0][0]) == float + assert type(window_ranges[0][1]) == float + assert window_ranges[0][0] == approx(1603122420.0) + assert window_ranges[0][1] == approx(1603122479.9) From 2289f561c1b447b3cf9d0ae8ab5e87adc84f5993 Mon Sep 17 00:00:00 2001 From: Erik Forsberg Date: Wed, 21 Oct 2020 13:05:35 +0200 Subject: [PATCH 4/4] Return a float from Cython implementation of HoppingWindow.ranges Fixes #674 --- faust/_cython/windows.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/faust/_cython/windows.pyx b/faust/_cython/windows.pyx index e2e442bda..ea8a66cd2 100644 --- a/faust/_cython/windows.pyx +++ b/faust/_cython/windows.pyx @@ -64,7 +64,7 @@ cdef class HoppingWindow: r = [] for start in range(int(start), int(timestamp) + 1, int(self.step)): end = start + self.size - 0.1 - r.append((start, end)) + r.append((float(start), end)) return r cdef double _start_initial_range(self, double timestamp):