napari / napari

napari: a fast, interactive, multi-dimensional image viewer for python
https://napari.org
BSD 3-Clause "New" or "Revised" License
2.21k stars 423 forks source link

Improve error message for saving attempt when no layer is selected #7387

Open melissawm opened 3 hours ago

melissawm commented 3 hours ago

πŸ› Bug Report

When using Ctrl+S (on Linux) to try to save a layer with no layer selected through the GUI, an OSError appears which is not very friendly to users.

πŸ’‘ Steps to Reproduce

  1. Create any layer on the gui
  2. Click Ctrl+S (Linux/Windows) or Command+S (Mac)
  3. Observe the OSError

πŸ’‘ Expected Behavior

Maybe a dialog with a friendlier "No layer selected. Please select a layer to save" message.

🌎 Environment

napari dev Manjaro Linux + KDE Python 3.11

πŸ’‘ Additional Context

Full traceback ``` --------------------------------------------------------------------------- OSError Traceback (most recent call last) File ~/micromamba/envs/napari-dev/lib/python3.11/site-packages/app_model/backends/qt/_qaction.py:57, in QCommandAction._on_triggered(self=QMenuItemAction(MenuItem(when=None, group='4_sav...tle=None, toggled=None), alt=None), app='napari'), checked=False) 53 def _on_triggered(self, checked: bool) -> None: 54 # execute_command returns a Future, for the sake of eventually being 55 # asynchronous without breaking the API. For now, we call result() 56 # to raise any exceptions. ---> 57 self._app.commands.execute_command(self._command_id).result() self._command_id = 'napari.window.file.save_layers_dialog.selected' self = QMenuItemAction(MenuItem(when=None, group='4_save', order=None, command=CommandRule(id='napari.window.file.save_layers_dialog.selected', title='Save Selected Layers...', category=None, tooltip=None, status_tip=None, icon=None, icon_visible_in_menu=True, enablement=Expr.parse('num_selected_layers > 0'), short_title=None, toggled=None), alt=None), app='napari') self._app = Application('napari') File ~/micromamba/envs/napari-dev/lib/python3.11/site-packages/app_model/registries/_commands_reg.py:245, in CommandsRegistry.execute_command(self=, id='napari.window.file.save_layers_dialog.selected', execute_asynchronously=False, *args=(), **kwargs={}) 241 except Exception as e: 242 if self._raise_synchronous_exceptions: 243 # note, the caller of this function can also achieve this by 244 # calling `future.result()` on the returned future object. --> 245 raise e 246 future.set_exception(e) 248 return future File ~/micromamba/envs/napari-dev/lib/python3.11/site-packages/app_model/registries/_commands_reg.py:240, in CommandsRegistry.execute_command(self=, id='napari.window.file.save_layers_dialog.selected', execute_asynchronously=False, *args=(), **kwargs={}) 238 future: Future = Future() 239 try: --> 240 future.set_result(cmd(*args, **kwargs)) future = cmd = args = () kwargs = {} 241 except Exception as e: 242 if self._raise_synchronous_exceptions: 243 # note, the caller of this function can also achieve this by 244 # calling `future.result()` on the returned future object. File ~/micromamba/envs/napari-dev/lib/python3.11/site-packages/in_n_out/_store.py:934, in Store.inject_processors.._deco.._exec(*args=(), **kwargs={}) 932 @wraps(func) 933 def _exec(*args: P.args, **kwargs: P.kwargs) -> R: --> 934 result = func(*args, **kwargs) func = args = () kwargs = {} 935 if result is not None: 936 self.process( 937 result, 938 type_hint=type_hint, (...) 941 _funcname=getattr(func, "__qualname__", str(func)), 942 ) File ~/micromamba/envs/napari-dev/lib/python3.11/site-packages/in_n_out/_store.py:804, in Store.inject.._inner.._exec(*args=(), **kwargs={}) 797 logger.debug( 798 " Calling %s with %r (injected %r)", 799 _fname, 800 bound.arguments, 801 _injected_names, 802 ) 803 try: --> 804 result = func(**bound.arguments) bound = )> func = bound.arguments = {'qt_viewer': } 805 except TypeError as e: 806 if "missing" not in e.args[0]: File ~/projects/napari/napari/_qt/_qapp_model/qactions/_file.py:105, in _save_selected_layers(qt_viewer=) 104 def _save_selected_layers(qt_viewer: QtViewer): --> 105 qt_viewer._save_layers_dialog(selected=True) qt_viewer = File ~/projects/napari/napari/_qt/qt_viewer.py:756, in QtViewer._save_layers_dialog(self=, selected=True) 751 msg = trans._( 752 'Please select one or more layers to save,' 753 '\nor use "Save all layers..."' 754 ) 755 if msg: --> 756 raise OSError(trans._('Nothing to save')) trans = 758 # prepare list of extensions for drop down menu. 759 ext_str, writers = _extension_string_for_layers( 760 list(self.viewer.layers.selection) 761 if selected 762 else self.viewer.layers 763 ) OSError: Nothing to save ```
willingc commented 3 hours ago

Great catch @melissawm and good suggestion.

willingc commented 3 hours ago

Looking at the traceback it seems as if there is a message created but it's not displayed to the user and sent to error handling.

File ~/Code/repos-napari/napari/napari/_qt/qt_viewer.py:756, in QtViewer._save_layers_dialog(self=<napari._qt.qt_viewer.QtViewer object>, selected=True)
    751     msg = trans._(
    752         'Please select one or more layers to save,'
    753         '\nor use "Save all layers..."'
    754     )
    755 if msg:
--> 756     raise OSError(trans._('Nothing to save'))
psobolewskiPhD commented 3 hours ago

Interesting, on macOS if I have no layer selected, the menu item is greyed-out and Command-S does nothing: no error, no warning. Odd that it behaves differently with different OS. @lucyleeow any ideas?

willingc commented 2 hours ago

I was able to reproduce on macOS sequoia when using the keyboard shortcut (on main not released version)