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

Moving points does not emit data change event #2260

Open
jwindhager opened this issue Feb 12, 2021 · 4 comments
Open

Moving points does not emit data change event #2260

jwindhager opened this issue Feb 12, 2021 · 4 comments
Labels
bug Something isn't working
Milestone

Comments

@jwindhager
Copy link
Contributor

🐛 Bug

Moving points in the viewer does not emit Points.events.data event

To Reproduce

Steps to reproduce the behavior:

  1. Add a points layer
  2. Register a callback for Points.events.data
  3. In the viewer, select and drag points and observe how the event is not emitted

Expected behavior

To my understanding, Points.events.data should be emitted whenever the data changes, so either

  • while the points are being moved (mouse drag), or
  • after the points have been moved (mouse release)

Additional context

A workaround is possible by additionally listening to mouse drag events. However, it this case, it is unclear how to differentiate between point selection and point move, see #2259.

@sofroniewn sofroniewn added this to the 0.4 milestone Feb 12, 2021
@sofroniewn sofroniewn added the bug Something isn't working label Feb 12, 2021
@sofroniewn
Copy link
Contributor

Thanks for filing @jwindhager - I know @jni has flagged this too. It is because we modify the data in place so the setter isn't called on the data property. We could just manually call the event here though

self.data[np.ix_(index, disp)] = (
self.data[np.ix_(index, disp)] + shift
)
which would be an easy one line fix as we work on a more elegant solution

@jwindhager
Copy link
Contributor Author

Thanks! Happy to submit a pull request for this. Just want to make sure that this doesn't break things? For example, if some existing plugins do some heavy lifting in the Points.events.data event handler (e.g. I/O), it might be a suboptimal idea to now change the behavior to emit this event during a mouse drag operation... Maybe there should be two separate events, e.g. data_updating and data_updated or something of that sort?

@sofroniewn
Copy link
Contributor

That's a reasonable concern, we could do something a little more complex like use a recently added ability to block and count event emission, see #2197. We would add event emission to the points move function after the line I indicated above, but then inside the mouse function we could block event emission when the mouse gets pressed, then when the mouse gets release we could count the number of emitted events, and if greater than 0 we could emit one manually there. Thoughts? cc @jni

@jni
Copy link
Member

jni commented Feb 15, 2021

Indeed, affinder specifically writes data each time the affine matrix changes, so this would re-save the affine matrix at each increment. It's tiny but I think that would add up. I like @sofroniewn's proposed solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants