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 Item Insert - negative indexing #4578

Closed
Durman opened this issue Jul 19, 2022 · 9 comments
Closed

List Item Insert - negative indexing #4578

Durman opened this issue Jul 19, 2022 · 9 comments
Labels

Comments

@Durman
Copy link
Collaborator

Durman commented Jul 19, 2022

Problem statement

It looks like a bug that -1 index does not refer to the last element.

image

#3827

@zeffii
Copy link
Collaborator

zeffii commented Jul 19, 2022

in the List Item Insert node, handles incoming data differently when list vs numpy.ndarray

image

@zeffii
Copy link
Collaborator

zeffii commented Jul 19, 2022

for ind, i in zip(*params):
if self.replace and len(data_out) > ind:
data_out.pop(ind)
data_out.insert(ind, i)

@zeffii zeffii mentioned this issue Jul 19, 2022
@Durman
Copy link
Collaborator Author

Durman commented Jul 19, 2022

The approach is not efficient for replace mode. Should be data_out[ind] = i

@zeffii
Copy link
Collaborator

zeffii commented Jul 19, 2022

for ind, i in zip(*params):
idx = ind % len(data) # translate to positive idx
if self.replace:
data_out[idx] = i
else:
data_out.insert(idx, i)

@zeffii zeffii closed this as completed Jul 19, 2022
@Durman
Copy link
Collaborator Author

Durman commented Jul 19, 2022

Or like this =). But this, I think, does not give big performance improvement, at least compare to moving from O(len(data)) to O(1).

if self.replace:
    for ind, i in zip(*params):
        idx = ind % len(data)  # translate to positive idx
        data_out[idx] = i
else:
    for ind, i in zip(*params):
        idx = ind % len(data)  # translate to positive idx
        data_out.insert(idx, i)

@zeffii
Copy link
Collaborator

zeffii commented Jul 19, 2022

hehe .. or that. commit if you wish :)

@Durman
Copy link
Collaborator Author

Durman commented Sep 6, 2022

image

It seems another bug was produced by the fix. I guess next string is to blame

idx = ind % len(data) # translate to positive idx

@Durman Durman reopened this Sep 6, 2022
@Durman
Copy link
Collaborator Author

Durman commented Sep 12, 2022

7cb3450

@Durman Durman closed this as completed Sep 12, 2022
@Durman
Copy link
Collaborator Author

Durman commented Sep 22, 2022

The fix changes behavior in some cases. It seems that previous behavior is not valid in the example

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants