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

key binding dispatch implementation #6204

Open
wants to merge 115 commits into
base: main
Choose a base branch
from
Open

key binding dispatch implementation #6204

wants to merge 115 commits into from

Conversation

kne42
Copy link
Member

@kne42 kne42 commented Sep 4, 2023

status:

  • update changes from enable npe2 keybinding contributions #5338 to use as base branch
  • fix various bugs due to merge conflicts
  • add dependency injection providers so that key bindings can run on old system
  • add base implementation adapted from NAP 7 for dispatch
  • add custom key binding registry
  • get context/expression evaluation to work correctly
  • create robust test suite for napari.utils.key_bindings
  • create robust test suite for napari._qt._qapp_model.QKeyBindingDispatcher
  • deprecate old key dispatch system
  • add workarounds for old bind_key methods to autopopulate new system
  • deprecate old action manager system where key bindings are used
  • adjust GUI population method
  • adjust GUI setting method
  • link ShortcutSettings to modify KeyBindingsRegistry
  • properly handle AttrRestoreCallbacks
  • fix failing pr checks

see NAP 7 for more information

kne42 and others added 30 commits November 14, 2022 16:42
…3/napari into keybinding-integration

� Conflicts:
�	napari/_app_model/_app.py
'kne42:keybind-contribution'

* aganders3-app-model-action-manager-rebase:
  Finish porting layer actions to app-model with attribute-restoring callback
  Add labels and shapes commands - mode commands still TODO
  Update/fix after rebase on main
  Update callback name when registering command
  Add GeneratorCallback wrapper for generator callbacks in app-model commands.
  Initial refactor of image and points actions into app-model
  Move viewer actions callbacks back to _viewer_key_bindings.py
  Move actions from _viewer_key_bindings to app-model
  fix: add disposer for repeatable action registry
  feature: keep track of which app-model actions can be repeated
kne42 and others added 3 commits October 25, 2023 09:49
* main: (59 commits)
  Fix permission for JasonEtco/create-an-issue@v2 action (napari#6370)
  Ensure the conda bundle workflows have the appropriate permissions (napari#6379)
  Exclude the loaded property when linking two layers (napari#6377)
  Automatic add `maintenance` label to upgrade dependecies workflow (napari#6371)
  Pydantic 2 compatibility using `pydantic.v1` (napari#6358)
  Update `fsspec`, `imageio`, `magicgui`, `napari-console`, `npe2`, `pillow`, `tensorstore`, `xarray` (napari#6367)
  fix `TransformChain.__getitem__` signature (napari#6369)
  Fix some typing in napari._vispy (napari#6245)
  Fix typing in napari.utils.theme (napari#6247)
  Fix typing in napari.layers.base (napari#6028)
  Fix build wheel workflow file name (napari#6357)
  Fix upstream deprecation: change `np.alltrue` to `np.array_equiv` (napari#6347)
  Add codecov token to reduce propability of fail to upload from main branch (napari#6346)
  Disable ptrace protection to make pytest-pystack work (napari#6343)
  Update example scripts (magicgui with threads) (napari#6353)
  Update `dask`, `hypothesis`, `numpy`, `pillow`, `pretend`, `psutil`, `pyqt5`, `pyqt6`, `pyside6`, `superqt` (napari#6322)
  Add vortex optical flow example (napari#6041)
  Enable memory benchmarks for layers (napari#6295)
  Fix labeler configuration (napari#6344)
  Add 'raise' option back to `strict_qt` in `make_napari_viewer` (napari#6335)
  ...
Co-authored-by: dalthviz <16781833+dalthviz@users.noreply.github.com>

app = get_app()

kb = coerce_keybinding(key_bind)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just in case, checking locally the error from the CI, I think adding here a call to the to_int method could help. So something like:

Suggested change
kb = coerce_keybinding(key_bind)
kb = coerce_keybinding(key_bind).to_int()

self.keybindings.register_keybinding_rule(
entry.command,
KeyBindingRule(
primary=KeyBinding.from_str(entry.key),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like adding a call to the to_int method here can also help with pydantic validation errors. So something like:

Suggested change
primary=KeyBinding.from_str(entry.key),
primary=KeyBinding.from_str(entry.key).to_int(),

* main: (80 commits)
  Check in LabelColormap that fewer than 2**16 colors are requested (napari#6540)
  Fix label color shuffling by also updating colormap size (napari#6460)
  Add `_block_refresh()` to layers (napari#6525)
  MNT: Use `partial` in samples menu to avoid leaking  (napari#6538)
  Update performance and reduce memory usage for big Labels layer in direct color mode (napari#6439)
  Reset single step and decimals on reset range slider in popup (napari#6523)
  Add copy operator to fix memory benchmarks (napari#6530)
  Restore quit shortcut (napari#6526)
  Fix problem with invalidate cache  (napari#6520)
  [pre-commit.ci] pre-commit autoupdate (napari#6505)
  Pass key event from Main window to our internal mechanism (napari#6419)
  Typing: Fully type 5 more files (napari#6361)
  Do not use native dialog for reset shortcuts (napari#6493)
  Use views rather than CPU-based hashing for 8- and 16-bit Labels data (napari#6467)
  Add import lint back to CI (napari#6506)
  Type napari.layers.image helper sub-modules (napari#6499)
  Bugfix: ensure gamma and opacity are floats (napari#6498)
  FIX: Remove `None` default from `_remove_dock_widget` (napari#6481)
  Add testing_extra and optional dependencies when creating constraints (napari#6487)
  Run test suite with optional dependencies and fix tests when `triangle` is installed (napari#6488)
  ...
* main:
  Use blobs instead of random integers (napari#6527)
  Set default dtype for empty `_ImageSliceResponse` (napari#6552)
  Add creating image from clipboard (napari#6532)
  Fix check if plugin is available from conda (napari#6545)
  Fix generation of layer creation functions docstrings (napari#6558)
  Print a whole stack if throttler is not finished (napari#6549)
  Update `app-model`, `babel`, `coverage`, `dask`, `fsspec`, `hypothesis`, `imageio`, `ipython`, `lxml`, `magicgui`, `pandas`, `pint`, `psutil`, `pydantic`, `pyqt6`, `pytest-qt`, `tensorstore`, `tifffile`, `virtualenv`, `xarray` (napari#6478)
  Minimal changes for mlx (and jax) compatibility for Image layers (napari#6553)
  [pre-commit.ci] pre-commit autoupdate (napari#6528)
  Moving IntensityVisualizationMixin from _ImageBase to Image (napari#6548)
* main:
  [Maint] Update build_docs workflow to match napari/docs (napari#6547)
  Add cache to Transform calculation (napari#6536)
* main:
  Fix labels mapping cache by filling it with background, not 0 (napari#6580)
  Simplify unused parameters of Quaternion functions. (napari#6424)
  Add size and ndim to LayerDataProtocol (napari#6494)
  Fix label direct mode for installation without numba (napari#6571)
  Fix test in napari_builtins to remove import from conftest (napari#6568)
  Remove `app-model!=0.2.4` from test constraints (napari#6577)
  Bump mypy version and fix errors (napari#6557)
  Update test to work with `app-model==0.2.4` (napari#6573)
  Added support for features in surface layers (napari#6515)
@kne42
Copy link
Member Author

kne42 commented Jan 16, 2024

Hey @napari/core-devs, what do you think is needed before we can merge this? It's becoming a bit troublesome to keep this updated as new commands are added that I need to merge in.

There also appears to be some random bug (currently re-running the test) but I'm not sure what it is/if it is related to this PR, anyone got ideas on what to do with it?

@Czaki
Copy link
Collaborator

Czaki commented Jan 16, 2024

This strange bug is unrelated and reported here #5443

I think that it could be merged just after finish 0.4.19 release. As we got last pending review it should be fast.

@dalthviz
Copy link
Member

Hi there, just in case, did a quick check locally and the only thing I noticed, regarding the shortcuts functionality from the GUI, is a curious behavior where setting the default keybinding for an action manually causes for the Alternative Keybinding column to also be set with the default value. Seems like it only happens when setting the action default keybinding/shortcut value 🤔 :

keybindings

@Czaki
Copy link
Collaborator

Czaki commented Feb 3, 2024

As 0.4.19 is released, could you resolve conflicts? Then ping me for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preferences Issues relating to the creation of new preference fields/panels qt Relates to qt tests Something related to our tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants