napari-imagej can't determine type hint for ArrayImg #56
Comments
|
Man, that is just a really picky op 😕 It's an Right now, PyImageJ will convert the python input to an I'd think we'd be able to convert a numpy I think we instead would have to "hide" these Ops, but that is also no easy task. I'm not sure what the best path forward is. What do you think, @hinerm? |
I agree that this isn't a problem for napari-imagej. I believe Fiji has the same problem when trying to run ops that require an input I don't think you need to explicitly hide these Ops in napari-imagej since the problem is consistent across environments. |
I agree that an issue should be filed in Ops-land, but what issue? There isn't really anything wrong with this Op; it is typed on One change we could make would be to write more Ops with the same signature but a broader input object, at a lower priority. This would allow you to get the functionality across the board. But it would not fix the problem we are describing here, which is that we expect to run this Op but cannot because its input type is too specific. Making this change would not avoid the issue, it would just introduce more Ops and probably more confusion. (As an aside, it sucks to keep filing issues and adding ops in imagej-ops when imagej-ops2 is so close...) The more impactful change might be what is described in imagej/imagej-ops#643, because adding that typing would give more clarity and could tell us what can be run. Here's an idea: what if we wrote a Here's a draft: public class SearcherWithPredicates implements Searcher {
public SearcherWithPredicates(Searcher s, List<? extends Predicate<SearchResult>> predicates) {
this.s = s;
this.uberPredicate = predicates.get(0);
for( i = 1; i < predicates.size(); i++)
this.uberPredicate = this.uberPredicate.and(predicates.get(i));
}
@Override
public List<SearchResult> search(final String text, final boolean fuzzy) {
return s.search(text).stream()
.filter(uberPredicate::test) // Only allow results that satisfy ALL predicates
.collect(Collections.toList());
}
} |
|
The core issue here is that ops are not meant to be run directly like this, but rather matched against compatible inputs. I do not think we should be exposing individual op signatures. Rather, when you execute a particular op, inputs should be fed to the Ops matcher as per the design, so the right one can be selected by the framework. Of course, for this to work, we do need to scan all the ops, and aggregate them into supported more general signatures of some kind. Otherwise, there is no way to know what (what types, how many) the user wants to feed to the matcher. It's a sort of chicken-and-egg problem. The simplest way I can think of to aggregate the ops is to collapse ops with same "simple" signatures into one entry. E.g. two theoretical ops |
|
A more complex way to collapse the ops would be:
|
Yup, this is exactly what I was getting at, and would be easily doable with that
I think this is easier said than done. We will have to figure out how to do that reduction. Which op is the one that gets chosen? You can't use priority. |
Yeah, my point is: you aren't choosing specific op implementations. When you present a "napari-native" op signature to the user, they then populate a corresponding magicgui widget, which is then fed to the ops matcher, not to any specific implementation. So issues of priority don't matter anymore (from the napari-imagej perspective). |
|
Yeah, that might work out. Will be a bit of work, though. Let's implement this on the Java side, though, in an extensible way? We also would want this behavior in Fiji... |
I agree that we want this behavior in Fiji, too! However, it's a bit tricky to generalize the "context-native" idea, especially across the Java/Python border. I guess on the Java side, we'd have all the napari-based Python types becoming some canonical Java type via So then we'd have an mechanism in Java that accepts a map from Java type to simple names, something like: And then for each op signature, for each input and output type, checks if that type There would be no need to remember, for each simplified signature, which op(s) generated it, although if we want to be clever we could keep a list of op implementations for which it applies, to display to the user in the search results for things like looking up source code. But this is extra credit. The simplified signatures need to be sufficiently well-typed to generate the magicgui widget. Then, once the user has populated the widget, you can call |
|
FWIW I did work isolating ops that operate on images and consolidating parameters in the op finder. |
Thanks @hinerm, I'll probably try to use as much of that as I can. It would be cool if we could migrate some of that to imagej-ops. |
|
Can we close this issue now, with the EDIT: Nope, we can't 😦 The op signature still shows up post-OpListings:
You get a clearer exception, which I pasted below. But I still dislike exposing this issue to users. I feel like I've mentioned this before, but I'm suprised that |
|
Closing this as duplicate of imagej/imagej-ops#648. You can reproduce in Fiji using the following script, yielding the following error: #@ Img input
#@ OpService ops
import net.imglib2.type.numeric.integer.IntType
ops.math().add(input, new IntType(10))Error: |
Running the
math.add(arg, value)op results in this exception when generating the widget panel:See #48 (comment)
The text was updated successfully, but these errors were encountered: