Distinguish between update_dims, extent changes, and refresh #5363
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5363 +/- ##
==========================================
+ Coverage 87.60% 89.08% +1.47%
==========================================
Files 584 584
Lines 49533 49535 +2
==========================================
+ Hits 43395 44126 +731
+ Misses 6138 5409 -729
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@Czaki : assigned you as a reviewer as you were last to change some of this code. I'm unsure if this refactor is a good idea, so feel free to push back if you don't! |
|
Looks ok. Is there any reason why it is WIP? |
I was a little unsure if I liked it, so I didn't want it be merged accidentally. But looking at it again and with a little positive sentiment from you, I marked it as ready to review. |
andy-sweet
changed the title
| if 'extent' in self.__dict__: | ||
| del self.extent |
There was a problem hiding this comment.
why is this attribute treated so weirdly rather than being set to None or similar? Maybe out of scope, but I just noticed this.
There was a problem hiding this comment.
This is how cached property works.
The whole mechanism is first to check if the name is defined in self.__dict__ and if no, then call the property and put the result in this dict.
None is the proper value, so it will not trigger a recalculation.
|
As this is a fairly small refactor/rename with some eyes on it and an approval, I'll merge this after 24 hours unless anyone objects. |
…5363) * Replace some usage of _update_dims with refresh * Use extent event instead of set_data
…5363) * Replace some usage of _update_dims with refresh * Use extent event instead of set_data
* Replace some usage of _update_dims with refresh * Use extent event instead of set_data
* Replace some usage of _update_dims with refresh * Use extent event instead of set_data
* Replace some usage of _update_dims with refresh * Use extent event instead of set_data
* Replace some usage of _update_dims with refresh * Use extent event instead of set_data
* Replace some usage of _update_dims with refresh * Use extent event instead of set_data
Description
This reduces the use of
Layer._update_dims, which should only be used when the dimensionality of a layer (defined by the return value ofLayer._get_ndim) changes. That can always happen in the data setter, but can also happen at other times in some layers.It also introduces a simpler method (
Layer._clear_extent) that can be used instead of_update_dimsin many places. This method emits a new event (Layer.events.extent) that indicates a new value of the cached/derived propertyLayer.extentmay be available (i.e. when the extent cache is cleared). This is used to clear the layer list extent cache instead ofLayer.events.set_data, which I believe is intended to indicate the presence of a new slice (e.g. for vispy). This seems like a pretty solid pattern for a derived property likeextenthere.Type of change
This is a refactor that attempts to clarify what
Layer._update_dimsdoes, when it should be used, and when it shouldn't.References
This was motivated as part of the async slicing integration work in #5236 where these definitions become important.
How has this been tested?
Final checklist:
trans.to make them localizable.For more information see our translations guide.