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

Error when "Run ortho-plane" is selected during 3d inference #8

Closed
GenevieveBuckley opened this issue Mar 21, 2022 · 2 comments
Closed

Comments

@GenevieveBuckley
Copy link

I get an error during 3D inference, when the "Run ortho-plane" box is checked.

Steps to reproduce

  1. Start napari and load a 3D dataset (I've used the Lucchi dataset)
  2. Go to the napari "Plugins" menu, then "empanada-napari", and click "3D Inference"
  3. Check the box labelled "Run ortho-plane"
  4. Click the button to begin "3D Inference"

Error message

Traceback (most recent call last):
  File "/Users/genevieb/mambaforge/envs/napari-empanada-dev/lib/python3.9/site-packages/magicgui/widgets/_bases/value_widget.py", line 57, in _on_value_change
    self.changed.emit(value)
  File "psygnal/_signal.py", line 681, in psygnal._signal.SignalInstance.emit
  File "/Users/genevieb/mambaforge/envs/napari-empanada-dev/lib/python3.9/site-packages/magicgui/events.py", line 86, in _run_emit_loop
    cb(*args[:max_args])
  File "/Users/genevieb/mambaforge/envs/napari-empanada-dev/lib/python3.9/site-packages/magicgui/widgets/_function_gui.py", line 200, in _disable_button_and_call
    self.__call__()
  File "/Users/genevieb/mambaforge/envs/napari-empanada-dev/lib/python3.9/site-packages/magicgui/widgets/_function_gui.py", line 306, in __call__
    value = self._function(*bound.args, **bound.kwargs)
  File "/Users/genevieb/Documents/GitHub/empanada/empanada-napari/empanada_napari/_volume_inference.py", line 174, in widget
    widget.engine.update_params(
TypeError: update_params() missing 2 required positional arguments: 'merge_iou_thr' and 'merge_ioa_thr'

Why this error happens, and what we should do about it

This error happens because the update_params method for the Engine3d class is expecting to be passed values for merge_iou_thr and merge_ioa_thr.

A better solution might be to use keyword only arguments for update methods. Then if no keyword argument is passed in for a particular kwarg, you could have it fall back on the default value that was provided in the __init__ and stored as an attribute eg: self.attributename.

There might also be a clearner/nicer solution than this. I'm not sure what the recommended way to update multiple attributes at once is.

Since this affects me, I'm happy to put in a pull request, but we'd need to settle on the approach to take first.

@conradry
Copy link
Contributor

Thanks for catching this! Originally I had merge_iou_thr and merge_ioa_thr as changeable parameters before deciding that they were mostly unnecessary for users to tweak. When I removed them from the widget I forgot to update the update_params method.

For now (and unless there's a use case where they need to be changed) I'm just going to leave them hard-coded. Otherwise, I think that making them kwargs would be the way to go.

I pushed a new version (0.1.3) that won't have this problem. Hopefully, it works for you.

@GenevieveBuckley
Copy link
Author

I don't have an objection to those values being hard coded (at least, for now I don't think I'm likely to change them).

I've tried out version 0.13 and no longer see the error message, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants