Fix #674 by explicitly setting return type to float in Cython version of HoppingWindow.ranges()

.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

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):

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
 

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(

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

t/unit/windows/test_hopping_window.py

@@ -1,3 +1,4 @@
+from pytest import approx
 from faust.windows import HoppingWindow
 
 
@@ -59,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)

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