address PR comments

napari/utils/interactions.py

@@ -5,7 +5,7 @@
 
 from numpydoc.docscrape import FunctionDoc
 
-from ..utils.key_bindings import KeyBindingLike, coerce_keybinding
+from ..utils.key_bindings import KeyBinding, KeyBindingLike, coerce_keybinding
 from ..utils.translations import trans
 
 
@@ -244,15 +244,27 @@ def hello_world(layer, event):
     KEY_SYMBOLS.update({'Meta': 'Super'})
 
 
-def kb2mods(kb) -> List[str]:
+def _kb2mods(key_bind: KeyBinding) -> List[str]:
+    """Extract list of modifiers from a key binding.
+
+    Parameters
+    ----------
+    key_bind : KeyBinding
+        The key binding whose mods are to be extracted.
+
+    Returns
+    -------
+    list of str
+        The key modifiers used by the key binding.
+    """
     mods = []
-    if kb.ctrl:
+    if key_bind.ctrl:
         mods.append('Ctrl')
-    if kb.shift:
+    if key_bind.shift:
         mods.append('Shift')
-    if kb.alt:
+    if key_bind.alt:
         mods.append('Alt')
-    if kb.meta:
+    if key_bind.meta:
         mods.append('Meta')
     return mods
 
@@ -270,12 +282,10 @@ class Shortcut:
     """
 
     def __init__(self, shortcut: KeyBindingLike):
-        """
-        Parameters
+        """Parameters
         ----------
         shortcut : keybinding-like
             shortcut to format
-
         """
         error_msg = trans._(
             "{shortcut} does not seem to be a valid shortcut Key.",
@@ -290,23 +300,26 @@ def __init__(self, shortcut: KeyBindingLike):
         else:
             for part in self._kb.parts:
                 shortcut_key = str(part.key)
-                if (
-                    len(shortcut_key) > 1
-                    and shortcut_key not in KEY_SYMBOLS.keys()
-                ):
+                if len(shortcut_key) > 1 and shortcut_key not in KEY_SYMBOLS:
                     error = True
 
         if error:
             warnings.warn(error_msg, UserWarning, stacklevel=2)
 
     @property
     def qt(self) -> str:
+        """Representation of the keybinding as it would appear in Qt.
+
+        Returns
+        -------
+        string
+            Shortcut formatted to be used with Qt.
+        """
         return str(self._kb)
 
     @property
     def platform(self) -> str:
-        """
-        Format the given shortcut for the current platform.
+        """Format the given shortcut for the current platform.
 
         Replace Cmd, Ctrl, Meta...etc by appropriate symbols if relevant for the
         given platform.
@@ -319,7 +332,7 @@ def platform(self) -> str:
         return ' '.join(
             joinchar.join(
                 KEY_SYMBOLS.get(x, x)
-                for x in (kb2mods(part) + [str(part.key)])
+                for x in (_kb2mods(part) + [str(part.key)])
             )
             for part in self._kb.parts
         )

napari/utils/key_bindings.py

@@ -63,7 +63,7 @@ def hello_world(viewer):
     'Option': 'Alt',
 }
 
-UNDEFINED = object()
+_UNDEFINED = object()
 
 _VISPY_SPECIAL_KEYS = [
     keys.SHIFT,
@@ -110,30 +110,30 @@ def hello_world(viewer):
 KeyBinding.__hash__ = lambda self: hash(str(self))
 
 
-def coerce_keybinding(kb: KeyBindingLike) -> KeyBinding:
+def coerce_keybinding(key_bind: KeyBindingLike) -> KeyBinding:
     """Convert a keybinding-like object to a KeyBinding.
 
     Parameters
     ----------
-    kb : keybinding-like
+    key_bind : keybinding-like
         Object to coerce.
 
     Returns
     -------
-    kb : KeyBinding
+    key_bind : KeyBinding
         Object as KeyBinding.
     """
-    if isinstance(kb, str):
+    if isinstance(key_bind, str):
         for k, v in KEY_SUBS.items():
-            kb = kb.replace(k, v)
+            key_bind = key_bind.replace(k, v)
 
-    return KeyBinding.validate(kb)
+    return KeyBinding.validate(key_bind)
 
 
 def bind_key(
     keymap: Keymap,
-    kb: Union[KeyBindingLike, EllipsisType],
-    func=UNDEFINED,
+    key_bind: Union[KeyBindingLike, EllipsisType],
+    func=_UNDEFINED,
     *,
     overwrite=False,
 ):
@@ -143,7 +143,7 @@ def bind_key(
     ----------
     keymap : dict of str: callable
         Keymap to modify.
-    kb : keybinding-like or ...
+    key_bind : keybinding-like or ...
         Key combination.
         ``...`` acts as a wildcard if no key combinations can be matched
         in the keymap (this will overwrite all key combinations
@@ -196,27 +196,27 @@ def hello_world(viewer):
 
     To create a keymap that will block others, ``bind_key(..., ...)```.
     """
-    if func is UNDEFINED:
+    if func is _UNDEFINED:
 
         def inner(func):
-            bind_key(keymap, kb, func, overwrite=overwrite)
+            bind_key(keymap, key_bind, func, overwrite=overwrite)
             return func
 
         return inner
 
-    if kb is not Ellipsis:
-        kb = coerce_keybinding(kb)
+    if key_bind is not Ellipsis:
+        key_bind = coerce_keybinding(key_bind)
 
-    if func is not None and kb in keymap and not overwrite:
+    if func is not None and key_bind in keymap and not overwrite:
         raise ValueError(
             trans._(
                 'keybinding {key} already used! specify \'overwrite=True\' to bypass this check',
                 deferred=True,
-                key=str(kb),
+                key=str(key_bind),
             )
         )
 
-    unbound = keymap.pop(kb, None)
+    unbound = keymap.pop(key_bind, None)
 
     if func is not None:
         if func is not Ellipsis and not callable(func):
@@ -226,7 +226,7 @@ def inner(func):
                     deferred=True,
                 )
             )
-        keymap[kb] = func
+        keymap[key_bind] = func
 
     return unbound
 
@@ -242,12 +242,14 @@ def _get_user_keymap() -> Keymap:
     return USER_KEYMAP
 
 
-def _bind_user_key(key: KeyBindingLike, func=UNDEFINED, *, overwrite=False):
+def _bind_user_key(
+    key_bind: KeyBindingLike, func=_UNDEFINED, *, overwrite=False
+):
     """Bind a key combination to the user keymap.
 
     See ``bind_key`` docs for details.
     """
-    return bind_key(_get_user_keymap(), key, func, overwrite=overwrite)
+    return bind_key(_get_user_keymap(), key_bind, func, overwrite=overwrite)
 
 
 def _vispy2appmodel(event) -> KeyBinding:
@@ -392,18 +394,18 @@ def active_keymap(self):
 
         return active_keymap_final
 
-    def press_key(self, kb):
+    def press_key(self, key_bind):
         """Simulate a key press to activate a keybinding.
 
         Parameters
         ----------
-        kb : keybinding-like
+        key_bind : keybinding-like
             Key combination.
         """
-        kb = coerce_keybinding(kb)
+        key_bind = coerce_keybinding(key_bind)
         keymap = self.active_keymap
-        if kb in keymap:
-            func = keymap[kb]
+        if key_bind in keymap:
+            func = keymap[key_bind]
         elif Ellipsis in keymap:  # catch-all
             func = keymap[...]
         else:
@@ -422,7 +424,7 @@ def press_key(self, kb):
 
         generator_or_callback = func()
 
-        key = str(kb.parts[-1].key)
+        key = str(key_bind.parts[-1].key)
 
         if inspect.isgeneratorfunction(func):
             try:
@@ -437,18 +439,18 @@ def press_key(self, kb):
                 time.time(),
             )
 
-    def release_key(self, kb):
+    def release_key(self, key_bind):
         """Simulate a key release for a keybinding.
 
         Parameters
         ----------
-        kb : keybinding-like
+        key_bind : keybinding-like
             Key combination.
         """
         from ..settings import get_settings
 
-        kb = coerce_keybinding(kb)
-        key = str(kb.parts[-1].key)
+        key_bind = coerce_keybinding(key_bind)
+        key = str(key_bind.parts[-1].key)
         with contextlib.suppress(KeyError, StopIteration):
             val = self._key_release_generators[key]
             # val could be callback function with time to check