Repository: ReactiveX/RxPY

Issue:
zip operator should complete if a single upstream source completes
The following observable `y` ...

``` python
import rx
from rx import operators as rxop

y = rx.range(2).pipe(
    rxop.zip(rx.defer(lambda _: y)),
    rxop.merge(rx.just(0)),
    rxop.share(),
)

y.subscribe(print, on_completed=lambda: print('completed'))
```

... never completes.

My suggestion: The zip operator should complete if any upstream source completed and the corresponding queue is empty.

That is how it is implemented in [RxJava](https://github.com/ReactiveX/RxJava/blob/3.x/src/main/java/io/reactivex/rxjava3/internal/operators/observable/ObservableZip.java).

Discussion:
coveralls: 
[![Coverage Status](https://coveralls.io/builds/32632137/badge)](https://coveralls.io/builds/32632137)

Coverage decreased (-0.01%) to 92.456% when pulling **c142cb18eb1d05e737b86c04ee158f6ba3e5ab04 on hoc081098:hoc081098/zip_completed** into **f2642a87e6b4d5b3dc9a482323716cf15a6ef570 on ReactiveX:master**.


Patch:
diff --git a/rx/__init__.py b/rx/__init__.py
index b995f2b2..ce18e050 100644
--- a/rx/__init__.py
+++ b/rx/__init__.py
@@ -6,7 +6,6 @@ from typing import Iterable, Callable, Any, Optional, Union, Mapping
 from .core import Observable, pipe, typing
 from .internal.utils import alias
 
-
 # Please make sure the version here remains the same as in project.cfg
 __version__ = '3.1.1'
 
@@ -561,8 +560,8 @@ def generate(initial_state: Any,
 
 
 def hot(string: str,
-        timespan: typing.RelativeTime=0.1,
-        duetime:typing.AbsoluteOrRelativeTime = 0.0,
+        timespan: typing.RelativeTime = 0.1,
+        duetime: typing.AbsoluteOrRelativeTime = 0.0,
         scheduler: Optional[typing.Scheduler] = None,
         lookup: Optional[Mapping] = None,
         error: Optional[Exception] = None
@@ -1092,8 +1091,8 @@ def zip(*args: Observable) -> Observable:
         :alt: zip
 
         --1--2---3-----4---|
-        -a----b----c-d-----|
-        [       zip()      ]
+        -a----b----c-d------|
+        [       zip()       ]
         --1,a-2,b--3,c-4,d-|
 
     Example:
diff --git a/rx/core/observable/zip.py b/rx/core/observable/zip.py
index 530ab8c8..8b756477 100644
--- a/rx/core/observable/zip.py
+++ b/rx/core/observable/zip.py
@@ -1,11 +1,11 @@
+from threading import RLock
 from typing import Optional, List
 
 from rx import from_future
 from rx.core import Observable, typing
 from rx.disposable import CompositeDisposable, SingleAssignmentDisposable
-from rx.internal.utils import is_future
 from rx.internal.concurrency import synchronized
-from threading import RLock
+from rx.internal.utils import is_future
 
 
 # pylint: disable=redefined-builtin
@@ -29,11 +29,11 @@ def _zip(*args: Observable) -> Observable:
 
     sources = list(args)
 
-    def subscribe(observer: typing.Observer, scheduler: Optional[typing.Scheduler] = None):
+    def subscribe(observer: typing.Observer,
+                  scheduler: Optional[typing.Scheduler] = None) -> CompositeDisposable:
         n = len(sources)
         queues: List[List] = [[] for _ in range(n)]
         lock = RLock()
-        is_done = [False] * n
 
         @synchronized(lock)
         def next(i):
@@ -46,13 +46,6 @@ def _zip(*args: Observable) -> Observable:
                     return
 
                 observer.on_next(res)
-            elif all([x for j, x in enumerate(is_done) if j != i]):
-                observer.on_completed()
-
-        def done(i):
-            is_done[i] = True
-            if all(is_done):
-                observer.on_completed()
 
         subscriptions = [None] * n
 
@@ -65,7 +58,7 @@ def _zip(*args: Observable) -> Observable:
                 queues[i].append(x)
                 next(i)
 
-            sad.disposable = source.subscribe_(on_next, observer.on_error, lambda: done(i), scheduler)
+            sad.disposable = source.subscribe_(on_next, observer.on_error, observer.on_completed, scheduler)
             subscriptions[i] = sad
 
         for idx in range(n):
diff --git a/rx/core/operators/tofuture.py b/rx/core/operators/tofuture.py
index 67197888..f0f1d435 100644
--- a/rx/core/operators/tofuture.py
+++ b/rx/core/operators/tofuture.py
@@ -38,18 +38,20 @@ def _to_future(future_ctor: Optional[Callable[[], Future]] = None,
             has_value = True
 
         def on_error(err):
-            future.set_exception(err)
+            if not future.cancelled():
+                future.set_exception(err)
 
         def on_completed():
             nonlocal last_value
-            if has_value:
-                future.set_result(last_value)
-            else:
-                future.set_exception(SequenceContainsNoElementsError())
+            if not future.cancelled():
+                if has_value:
+                    future.set_result(last_value)
+                else:
+                    future.set_exception(SequenceContainsNoElementsError())
             last_value = None
 
-        source.subscribe_(on_next, on_error, on_completed, scheduler=scheduler)
+        dis = source.subscribe_(on_next, on_error, on_completed, scheduler=scheduler)
+        future.add_done_callback(lambda _: dis.dispose())
 
-        # No cancellation can be done
         return future
     return to_future
diff --git a/rx/operators/__init__.py b/rx/operators/__init__.py
index c2373d76..b7da67a4 100644
--- a/rx/operators/__init__.py
+++ b/rx/operators/__init__.py
@@ -3492,8 +3492,8 @@ def zip(*args: Observable) -> Callable[[Observable], Observable]:
         :alt: zip
 
         --1--2---3-----4---|
-        -a----b----c-d-----|
-        [       zip()      ]
+        -a----b----c-d------|
+        [       zip()       ]
         --1,a-2,b--3,c-4,d-|
 
 


Test patch:
diff --git a/tests/test_observable/test_tofuture.py b/tests/test_observable/test_tofuture.py
index 679c92a2..f90fb111 100644
--- a/tests/test_observable/test_tofuture.py
+++ b/tests/test_observable/test_tofuture.py
@@ -6,6 +6,7 @@ import rx
 import rx.operators as ops
 from rx.internal.exceptions import SequenceContainsNoElementsError
 from rx.testing import ReactiveTest
+from rx.subject import Subject
 
 on_next = ReactiveTest.on_next
 on_completed = ReactiveTest.on_completed
@@ -79,3 +80,33 @@ class TestToFuture(unittest.TestCase):
 
         loop.run_until_complete(go())
         assert result == 42
+
+    def test_cancel(self):
+        loop = asyncio.get_event_loop()
+
+        async def go():
+            source = rx.return_value(42)
+            fut = next(source.__await__())
+            # This used to raise an InvalidStateError before we got
+            # support for cancellation.
+            fut.cancel()
+            await fut
+
+        self.assertRaises(asyncio.CancelledError, loop.run_until_complete, go())
+
+    def test_dispose_on_cancel(self):
+        loop = asyncio.get_event_loop()
+        sub = Subject()
+
+        async def using_sub():
+            # Since the subject never completes, this await statement
+            # will never be complete either. We wait forever.
+            await rx.using(lambda: sub, lambda s: s)
+
+        async def go():
+            await asyncio.wait_for(using_sub(), 0.1)
+
+        self.assertRaises(asyncio.TimeoutError, loop.run_until_complete, go())
+        # When we cancel the future (due to the time-out), the future
+        # automatically disposes the underlying subject.
+        self.assertTrue(sub.is_disposed)
diff --git a/tests/test_observable/test_zip.py b/tests/test_observable/test_zip.py
index 41eda422..fe09b461 100644
--- a/tests/test_observable/test_zip.py
+++ b/tests/test_observable/test_zip.py
@@ -13,7 +13,6 @@ disposed = ReactiveTest.disposed
 created = ReactiveTest.created
 
 
-
 class TestZip(unittest.TestCase):
 
     def test_zip_never_never(self):
@@ -37,7 +36,7 @@ class TestZip(unittest.TestCase):
             return o1.pipe(ops.zip(o2))
 
         results = scheduler.start(create)
-        assert results.messages == []
+        assert results.messages == [on_completed(210)]
 
     def test_zip_empty_empty(self):
         scheduler = TestScheduler()
@@ -67,7 +66,7 @@ class TestZip(unittest.TestCase):
                 ops.map(sum))
 
         results = scheduler.start(create)
-        assert results.messages == [on_completed(215)]
+        assert results.messages == [on_completed(210)]
 
     def test_zip_non_empty_empty(self):
         scheduler = TestScheduler()
@@ -82,7 +81,7 @@ class TestZip(unittest.TestCase):
                 ops.map(sum))
 
         results = scheduler.start(create)
-        assert results.messages == [on_completed(215)]
+        assert results.messages == [on_completed(210)]
 
     def test_zip_never_non_empty(self):
         scheduler = TestScheduler()
@@ -96,7 +95,7 @@ class TestZip(unittest.TestCase):
                 ops.map(sum))
 
         results = scheduler.start(create)
-        assert results.messages == []
+        assert results.messages == [on_completed(220)]
 
     def test_zip_non_empty_never(self):
         scheduler = TestScheduler()
@@ -110,7 +109,7 @@ class TestZip(unittest.TestCase):
                 ops.map(sum))
 
         results = scheduler.start(create)
-        assert results.messages == []
+        assert results.messages == [on_completed(220)]
 
     def test_zip_non_empty_non_empty(self):
         scheduler = TestScheduler()
@@ -125,7 +124,7 @@ class TestZip(unittest.TestCase):
                 ops.map(sum))
 
         results = scheduler.start(create)
-        assert results.messages == [on_next(220, 2 + 3), on_completed(240)]
+        assert results.messages == [on_next(220, 2 + 3), on_completed(230)]
 
     def test_zip_empty_error(self):
         ex = 'ex'
@@ -246,6 +245,7 @@ class TestZip(unittest.TestCase):
             for i in range(5):
                 results.append(on_next(205 + i * 5, i))
             return results
+
         msgs1 = msgs1_factory()
 
         def msgs2_factory():
@@ -253,6 +253,7 @@ class TestZip(unittest.TestCase):
             for i in range(10):
                 results.append(on_next(205 + i * 8, i))
             return results
+
         msgs2 = msgs2_factory()
 
         length = min(len(msgs1), len(msgs2))
@@ -265,13 +266,13 @@ class TestZip(unittest.TestCase):
                 ops.map(sum))
 
         results = scheduler.start(create).messages
-        assert(length == len(results))
+        assert (length == len(results))
         for i in range(length):
             _sum = msgs1[i].value.value + msgs2[i].value.value
             time = max(msgs1[i].time, msgs2[i].time)
-            assert(results[i].value.kind == 'N'
-                   and results[i].time == time
-                   and results[i].value.value == _sum)
+            assert (results[i].value.kind == 'N'
+                    and results[i].time == time
+                    and results[i].value.value == _sum)
 
     def test_zip_some_data_asymmetric2(self):
         scheduler = TestScheduler()
@@ -282,6 +283,7 @@ class TestZip(unittest.TestCase):
                 results.append(on_next(205 + i * 5, i))
 
             return results
+
         msgs1 = msgs1_factory()
 
         def msgs2_factory():
@@ -289,6 +291,7 @@ class TestZip(unittest.TestCase):
             for i in range(5):
                 results.append(on_next(205 + i * 8, i))
             return results
+
         msgs2 = msgs2_factory()
 
         length = min(len(msgs1), len(msgs2))
@@ -301,13 +304,13 @@ class TestZip(unittest.TestCase):
                 ops.map(sum))
 
         results = scheduler.start(create).messages
-        assert(length == len(results))
+        assert (length == len(results))
         for i in range(length):
             _sum = msgs1[i].value.value + msgs2[i].value.value
             time = max(msgs1[i].time, msgs2[i].time)
-            assert(results[i].value.kind == 'N'
-                   and results[i].time == time
-                   and results[i].value.value == _sum)
+            assert (results[i].value.kind == 'N'
+                    and results[i].time == time
+                    and results[i].value.value == _sum)
 
     def test_zip_some_data_symmetric(self):
         scheduler = TestScheduler()
@@ -317,6 +320,7 @@ class TestZip(unittest.TestCase):
             for i in range(10):
                 results.append(on_next(205 + i * 5, i))
             return results
+
         msgs1 = msgs1_factory()
 
         def msgs2_factory():
@@ -324,6 +328,7 @@ class TestZip(unittest.TestCase):
             for i in range(10):
                 results.append(on_next(205 + i * 8, i))
             return results
+
         msgs2 = msgs2_factory()
 
         length = min(len(msgs1), len(msgs2))
@@ -336,13 +341,13 @@ class TestZip(unittest.TestCase):
                 ops.map(sum))
 
         results = scheduler.start(create).messages
-        assert(length == len(results))
+        assert (length == len(results))
         for i in range(length):
             _sum = msgs1[i].value.value + msgs2[i].value.value
             time = max(msgs1[i].time, msgs2[i].time)
-            assert(results[i].value.kind == 'N'
-                   and results[i].time == time
-                   and results[i].value.value == _sum)
+            assert (results[i].value.kind == 'N'
+                    and results[i].time == time
+                    and results[i].value.value == _sum)
 
     def test_zip_with_iterable_never_empty(self):
         scheduler = TestScheduler()
@@ -423,7 +428,7 @@ class TestZip(unittest.TestCase):
     def test_zip_with_iterable_non_empty_non_empty(self):
         scheduler = TestScheduler()
         n1 = scheduler.create_hot_observable(
-                on_next(150, 1), on_next(215, 2), on_completed(230))
+            on_next(150, 1), on_next(215, 2), on_completed(230))
         n2 = [3]
 
         def create():