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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: Points.events.data is no longer fired from the data setter #6116

Closed
jni opened this issue Aug 4, 2023 · 1 comment 路 Fixed by #6117
Closed

Regression: Points.events.data is no longer fired from the data setter #6116

jni opened this issue Aug 4, 2023 · 1 comment 路 Fixed by #6117
Labels
bug Something isn't working priority-high High priority issue
Milestone

Comments

@jni
Copy link
Member

jni commented Aug 4, 2023

馃悰 Bug

While investigating affinder CI failures, I found that #5967 broke affinder's tests. (btw if you've never heard of git bisect run you should try it! 馃槏) The reason is this line which removes the data event emitter from the setter but doesn't replace it with the new style.

It seems to have been simply missed. @melonora was this intentional? I can't imagine the reason for it other than an oversight! 馃槄

Fix incoming.

To Reproduce

import napari
import numpy as np


layer = napari.layers.Points(np.random.random((5, 3)))

layer.events.data.connect(lambda ev: print(ev))

print('adding a single point')
layer.add(np.random.random((1, 3)))  # prints ok

print('setting data')
layer.data = np.random.random((8, 3))  # nothing happens

print('done')

Result:

adding a single point
Event
setting data
done

Expected result (and result before 96516da):

adding a single point
Event
setting data
Event
done

Environment

napari main / napari 0.4.18.

@jni jni added the bug Something isn't working label Aug 4, 2023
@jni jni added this to the 0.4.19 milestone Aug 4, 2023
@melonora
Copy link
Contributor

melonora commented Aug 4, 2023

oops 馃槻, this I believe is indeed an oversight. For sure it was not intentional.

jni added a commit to jni/affinder that referenced this issue Aug 4, 2023
@jni jni added the priority-high High priority issue label Aug 5, 2023
melonora pushed a commit that referenced this issue Aug 8, 2023
Closes #6116

# Description

Adds back an event emission from the Points layer data setter. Commits
summary:

- Add failing test for layer data setter event
- Emit event on points data setter

## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)

---------

Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Czaki added a commit that referenced this issue Oct 17, 2023
Closes #6116

# Description

Adds back an event emission from the Points layer data setter. Commits
summary:

- Add failing test for layer data setter event
- Emit event on points data setter

## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)

---------

Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority-high High priority issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants