Fix some typing in napari.components#6203
Fix some typing in napari.components#6203melonora merged 5 commits intonapari:mainfrom dstansby:components-typing
Conversation
Codecov Report
@@ Coverage Diff @@
## main #6203 +/- ##
=======================================
Coverage 91.67% 91.68%
=======================================
Files 583 583
Lines 51203 51205 +2
=======================================
+ Hits 46942 46947 +5
+ Misses 4261 4258 -3
|
andy-sweet
left a comment
There was a problem hiding this comment.
Thanks for the latest typing updates! I'm pleasantly surprised that most of _layer_slicer passed typing checks, but not surprised on the needed updates - I made an alternative suggestion there that I think expresses the intent more clearly and simply.
Otherwise, looks good!
napari/components/_layer_slicer.py
Outdated
| _SliceRequest = Callable[[], _SliceResponse] | ||
|
|
||
|
|
||
| class _SliceRequest(abc.ABC, Generic[_SliceResponse]): |
There was a problem hiding this comment.
I'd prefer this be a protocol instead of a generic because this ABC isn't actually inherited anywhere. See https://github.com/dstansby/napari/pull/3 for my suggestion for doing that keeps mypy happy and also simplifies the typing here.
| if previous_mode != (pan_zoom := selected_layer._modeclass.PAN_ZOOM): # type: ignore[attr-defined] | ||
| selected_layer.mode = pan_zoom |
There was a problem hiding this comment.
Nit: I think the walrus operator hurts readability here over the previous form because the reader has to unpack the two expressions in the condition now. I guess you need two ignores in that case though, so I don't care too much.
More generally, typing of Mode feels a bit broken and this is just one symptom. Do we have an issue to track a longer term fix for that?
There was a problem hiding this comment.
I agree. This walrus operator is obsolete as this part of the code does not contain any heavy calculations that need to be cached.
There was a problem hiding this comment.
I unpacked the walrus operator and added a comment to explain why we need the type: ignore.
We do have an issue to track the more general problem with layer modes: #6005
|
Since, this is pretty small, I'll probably merge this after the weekend. |
* main: (26 commits) Fix some typing in napari.components (napari#6203) Use class name for object that does not have qt name (napari#6222) test: [Automatic] Constraints upgrades: `hypothesis`, `magicgui`, `psygnal`, `tensorstore`, `tifffile`, `tqdm`, `virtualenv` (napari#6143) Replace more np.all( ... = ...) with np.array_equal (napari#6213) remove np.all(... == ...) in test_surface.py (napari#6218) Ensure pandas Series is initialized with a list as data (napari#6226) Stop using temporary directory for store array for paint test (napari#6191) Bugfix: ensure thumbnail represents canvas when multiscale (napari#6200) cleanup np.all(... == ...) from test_points.py (napari#6217) [pre-commit.ci] pre-commit autoupdate (napari#6221) use app-model for file menu (napari#4865) Add tests to cover slicing behavior when changing layers and data (napari#4819) [pre-commit.ci] pre-commit autoupdate (napari#6128) Add test coverage for async slicing of labels (napari#5325) Add collision check when set colors for labels layer (napari#6193) Update "toggle ndview" text (napari#6192) Prevent layer controls buttons changing layout while taking screenshots with flash effect on (napari#6194) Fix typing in napari.utils.perf (napari#6132) Add GUI test coverage for changes to Labels.show_selected_label (napari#5372) Fix types in 'napari.utils.colormaps.categorical_colormap' (napari#6154) ...
#6203 was merged without updating the latest changes from `main` and now the typing checks in PRs fails due to updates in `napari.components`. We don't run those changes on `main`, though maybe we should given that we don't requires PR branches to be up-to-date before merging (only that git reports no conflicts). This PR fixes the typing failure by casting the type away from the possible value of `None`, which is a side-effect of calling `_open_preferences_dialog`. An alternative fix is to have `_open_preferences_dialog` to return `PreferencesDialog`, which I like but felt like a bigger change than needed. Of course, we can also use `# type: ignore`, but I don't there's a need here.
Description
Fixes some typing in
napari.components.