Fix broken dims order popup and add to 3D #7937
Conversation
|
@psobolewskiPhD yay renaming! yay 3D!❤️ |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7937 +/- ##
==========================================
+ Coverage 92.90% 92.92% +0.02%
==========================================
Files 643 643
Lines 60673 60679 +6
==========================================
+ Hits 56369 56388 +19
+ Misses 4304 4291 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -85,3 +87,6 @@ def _dims_order_callback(self, event): | |||
| # Regenerate AxisList upon Dims side order changes for easy cleanup | |||
| self.axis_list = AxisList.from_dims(self.dims) | |||
There was a problem hiding this comment.
Just so I am understanding: when self.axis_list is regenerated here, the callback created in line 74/76 above is destroyed and isn't recreated, right?
So that's why after you change the order it fails to work.
But if you exist out and re-open, we run the init which makes a new self.axis_list and creates the callback.
So basically swapping works for one shot with a given QtDimsSorter currently and this makes it work multiple times per Sorter.
There was a problem hiding this comment.
Bam! You got it. The axis_list is overwritten, so the event is destroyed (or at least stale, I don't actually know enough about the technicals of Python to know if its totally gone, but I would think so). This test addition reveals that (in a non-intuitive way)
On main, the len(... .callbacks) == 1 instead of 2.
I don't also know if this is 'technically' the correct implementation (something feels weird about it), but it does re-add the destroyed event to the new axis_list, which is what we needed
There was a problem hiding this comment.
thanks for the explanation.
I did some checking locally and never got more than 2 callbacks so it seems to be behaving as intended.
|
I made a docs PR with a tip about the renaming: napari/docs#709 |
|
For release notes |
References and relevant issues
Closes #7916
Fixes #7928 (review)
Based on discussion in #7928
Description