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

Computer ops do not prevent user from selecting out == in #233

Open
ctrueden opened this issue May 23, 2023 · 3 comments
Open

Computer ops do not prevent user from selecting out == in #233

ctrueden opened this issue May 23, 2023 · 3 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@ctrueden
Copy link
Member

ctrueden commented May 23, 2023

When running a computer op, napari-imagej does not issue an error message when specifying the same layer for both input and output. But some computer ops fail when attempting to use the same image for input and output. Example steps to reproduce:

  1. Launch napari-imagej via Plugins → ImageJ2 (napari-imagej)
  2. Open an image via File → Open File(s)... – I used NESb_C2_TP1.tiff from NESb.zip of the SCIFIO sample images
  3. Type gauss into the napari-imagej search bar
  4. Expand the Ops (13) tree branch
  5. Double click filter.gauss(img "out", img "in", double[] "sigmas") -> (img "out")
  6. Note that the "out" and "in" point to the same layer
@gselzer gselzer self-assigned this May 23, 2023
@gselzer
Copy link
Collaborator

gselzer commented May 23, 2023

It seems like that would be pretty tricky to implement in practice.

Right now, napari-imagej delegates completely to magicgui to make the layer selector boxes. As such, any solution would require overriding the layer ComboBoxes.

I think that if we did this we'd need to add an I/O indicator field to each selector. Then, for each layer, we'd have to pass the Layer choices function to each ComboBox that is created by a module. More specifically, it should probably be a Callable object that knows whether it is an input or output ComboBox, along with a callback mechanism that other ComboBoxes can use to identify the Layers that are off-limits.

While I bet it's possible, it certainly hurts my brain to think about. Do you envision an easier way to do this, @ctrueden?

@gselzer gselzer added this to the 1.0.0 milestone May 23, 2023
@gselzer gselzer added the bug Something isn't working label May 23, 2023
@ctrueden
Copy link
Member Author

Yeah, we can just do post-dismissal validation, similar to how we handle the settings dialog. If it's a computer op and the in == out, display an error dialog and do not proceed with the execution.

@gselzer
Copy link
Collaborator

gselzer commented May 26, 2023

Yeah, we can just do post-dismissal validation, similar to how we handle the settings dialog. If it's a computer op and the in == out, display an error dialog and do not proceed with the execution.

You know @ctrueden, since this is a problem outside of napari-imagej, maybe this validation should be added to the OpListingModule upstream? Then it would be fixed everywhere...

EDIT: Although, it may be tough to ensure that the same napari layer isn't both input and output within Ops. I don't know whether scyjava returns the same UnsafeImg object for multiple calls to ij.py.to_java(<py image>), or if those UnsafeImgs would be considered equal if they aren't the same object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants