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

Consistent API for layer.data.events in setter.data #6125

Closed
melonora opened this issue Aug 7, 2023 · 0 comments 路 Fixed by #6134
Closed

Consistent API for layer.data.events in setter.data #6125

melonora opened this issue Aug 7, 2023 · 0 comments 路 Fixed by #6134
Assignees
Labels
maintenance PR with maintance changes, task
Milestone

Comments

@melonora
Copy link
Contributor

melonora commented Aug 7, 2023

馃О Task

With #6117 the events.data emitter is fixed. However, the API for this emitter in setter.data in Points is now different from setter.data in Shapes. The API should be made consistent.

@melonora melonora added task maintenance PR with maintance changes, labels Aug 7, 2023
@melonora melonora added this to the 0.4.19 milestone Aug 7, 2023
@melonora melonora self-assigned this Aug 7, 2023
melonora added a commit that referenced this issue Aug 18, 2023
# Fixes/Closes

Closes #6125 

# Description
With #6117 the emitted data in the `data.setter` of `Points` has been
made more consistent with the emitted data when adding, changing or
removing data within the layer. However, in `Shapes` only the data
itself was emitted. Furthermore,
since the `data.setter` in Shapes called the public `.add` method and
emits data itself, a `layers.data` event was emitted twice.
This PR is a slight refactor so that `data.setter` can directly call the
private `_add_shapes` method, bypassing the `layer.data` emit in `.add`.


## Type of change

- [x] Bug-fix (non-breaking change which fixes an issue)
- [x] Maintenance (changes required to run napari, tests, & CI smoothly)

# How has this been tested?

- [x] example: the test suite for my feature covers cases x, y, and z
- [x] example: all tests pass with my change
- [x] example: I check if my changes works with both PySide and PyQt
backends
      as there are small differences between the two Qt bindings.  

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Czaki pushed a commit that referenced this issue Oct 17, 2023
# Fixes/Closes

Closes #6125 

# Description
With #6117 the emitted data in the `data.setter` of `Points` has been
made more consistent with the emitted data when adding, changing or
removing data within the layer. However, in `Shapes` only the data
itself was emitted. Furthermore,
since the `data.setter` in Shapes called the public `.add` method and
emits data itself, a `layers.data` event was emitted twice.
This PR is a slight refactor so that `data.setter` can directly call the
private `_add_shapes` method, bypassing the `layer.data` emit in `.add`.


## Type of change

- [x] Bug-fix (non-breaking change which fixes an issue)
- [x] Maintenance (changes required to run napari, tests, & CI smoothly)

# How has this been tested?

- [x] example: the test suite for my feature covers cases x, y, and z
- [x] example: all tests pass with my change
- [x] example: I check if my changes works with both PySide and PyQt
backends
      as there are small differences between the two Qt bindings.  

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] If I included new strings, I have used `trans.` to make them
localizable.
For more information see our [translations
guide](https://napari.org/developers/translations.html).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance PR with maintance changes, task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant