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

list assignment index out of range error in zip_array class extension method #35

Closed
trilokacharya opened this issue Apr 3, 2015 · 10 comments

Comments

@trilokacharya
Copy link

The class extension method zip_array's action() method has this code:

for _ in queues:
    queues[n] = []

n is set to len(queues) earlier in the method, so this throws index out of range error.

@dbrattli
Copy link
Collaborator

dbrattli commented Apr 4, 2015

Thanks for pointing this out. That code looks ugly.

@trilokacharya
Copy link
Author

Thanks for jumping on it so quickly. Any idea when this would get merged into master?

@dbrattli
Copy link
Collaborator

dbrattli commented Apr 6, 2015

I don't have code that triggers the bug, so it's hard for me to know if really fixes the bug. If you could verify the fix, or post some minimal code that triggers the bug, then I can make a unit-test. When we know the fix is correct, then I can merge to master.

@38elements
Copy link
Contributor

Hi, @dbrattli
Hi, @trilokacharya
I tried this by using Python 2.7 and RxPY 1.1.
I reproduced the bug in RxPY 1.1 with the code below and I also confirmed the 72eecb fixes the problem.

from __future__ import print_function
import rx

s = rx.Observable.range(0, 5)
rx.Observable.zip_array(
    s,
    s.skip(1),
    s.skip(2)
).subscribe(print)

result

  File "/usr/local/lib/python2.7/dist-packages/rx/linq/observable/ziparray.py", line 82, in action
    queues[n] = []
IndexError: list assignment index out of range

@dbrattli
Copy link
Collaborator

dbrattli commented Apr 7, 2015

The code in Observable.zip and Observable.zip_arraylooks very similar. What is really the difference between zip_arrayand zip(s1, s2, lambda: x, y: [x, y]) @mattpodwysocki ? I checked the JavaScript sources, and they still have both operators Observable.zipArrayand observableProto.zip that are very similar. Would be nice if we could refactor this code, and implement zip_array (or really zip_list in python) just using zip.

@trilokacharya
Copy link
Author

@dbrattli, my understanding from reading the docs at http://reactivex.io/documentation/operators/zip.html is that zip_array is supposed to take a variable number of observables and zip them all.

This is not pretty, but I think it shows the intended behavior:

# Note: please excuse the formatting errors. I can't figure out how to get my formatting to stick in Markup
def flat_add(a,b):
        """ if a = valueA, b = valueB : returns [valueA, valueB]. 
            if a = [V1,V2..Vn], b = valueB : returns [V1,V2..Vn,valueB ]
        """
    if isinstance(a,list):
               a.append(b)
               return a
    else:
               return [a,b]

s1 = rx.Observable.range(0, 5)
s2 = s1.skip(1)
s3 = s1.skip(2)

reduce(lambda a,b:a.zip(b,flat_add),[s1,s2,s3])

"""
result = 
[0, 1, 2]
[1, 2, 3]
[2, 3, 4]
""""

@dbrattli
Copy link
Collaborator

dbrattli commented Apr 8, 2015

Yes, but you can do that already with zip:

>>> import rx
>>> def res(*args):
...    return list(args)
...
>>> s1 = rx.Observable.range(0, 5)
>>> s2 = s1.skip(1)
>>> s3 = s1.skip(2)
>>> rx.Observable.zip(s1, s2, s3, res).subscribe(print)
[0, 1, 2]
[1, 2, 3]
[2, 3, 4]
<rx.autodetachobserver.AutoDetachObserver object at 0x1025c1320>

Isn't this the same?

@trilokacharya
Copy link
Author

You're right. I should've read the description better instead of just looking at the diagram and thinking zip only works on 2 observables at a time.

dbrattli added a commit that referenced this issue Apr 11, 2015
Thus removed a lot of duplicated code. Also renamed it to zip_list
since we don’t have arrays in Python, but kept zip_array as an alias
for now.
@dbrattli
Copy link
Collaborator

I've now added the simplified Observable.zip_array to the develop branch.

@lock
Copy link

lock bot commented Jan 23, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants