From b952927729544cef3def279c8583ab86e54ace20 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Mon, 24 May 2021 11:16:23 -0500 Subject: [PATCH 01/24] Create Optional annotation --- .../main/java/org/scijava/param/Optional.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 scijava/scijava-ops/src/main/java/org/scijava/param/Optional.java diff --git a/scijava/scijava-ops/src/main/java/org/scijava/param/Optional.java b/scijava/scijava-ops/src/main/java/org/scijava/param/Optional.java new file mode 100644 index 000000000..160afdb92 --- /dev/null +++ b/scijava/scijava-ops/src/main/java/org/scijava/param/Optional.java @@ -0,0 +1,19 @@ + +package org.scijava.param; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Annotation to mark a parameter as optional: Ops with optional parameters + * should be callable with or without their Optional arguments + * + * @author Gabriel Selzer + */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.PARAMETER) +public @interface Optional { + +} From bba2d79a6fc300c742e3fc8dfd231d8ac1769d32 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 25 May 2021 14:34:59 -0500 Subject: [PATCH 02/24] OpInfo: add optional parameter methods These methods tell us about the presence of Optional parameters on the Op --- .../src/main/java/org/scijava/ops/OpInfo.java | 5 +++ .../scijava/ops/matcher/OpAdaptationInfo.java | 17 +++++++++ .../org/scijava/ops/matcher/OpClassInfo.java | 38 +++++++++++++++++++ .../org/scijava/ops/matcher/OpFieldInfo.java | 18 +++++++++ .../org/scijava/ops/matcher/OpMethodInfo.java | 14 +++++++ .../ops/simplify/SimplifiedOpInfo.java | 14 +++++++ 6 files changed, 106 insertions(+) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/OpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/OpInfo.java index bca5d202b..3a7cf54b4 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/OpInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/OpInfo.java @@ -2,6 +2,7 @@ package org.scijava.ops; import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; import java.util.List; @@ -67,6 +68,10 @@ default OpCandidate createCandidate(OpEnvironment env, Logger log, OpRef ref, Ma AnnotatedElement getAnnotationBearer(); + boolean hasOptionalParameters(); + + Parameter[] optionalParameters(); + @Override default int compareTo(final OpInfo that) { if (this.priority() < that.priority()) return 1; diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpAdaptationInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpAdaptationInfo.java index 5f630559e..55687ff64 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpAdaptationInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpAdaptationInfo.java @@ -1,6 +1,7 @@ package org.scijava.ops.matcher; import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.List; import java.util.function.Function; @@ -8,6 +9,7 @@ import org.scijava.ops.OpDependencyMember; import org.scijava.ops.OpInfo; import org.scijava.ops.OpUtils; +import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; import org.scijava.struct.Struct; @@ -105,4 +107,19 @@ public boolean isSimplifiable() { return false; } + @Override + public boolean hasOptionalParameters() { + return false; + } + + /** + * NB for {@link Optional} annotations to be on the Op, they would have to be + * declared within the adapter. Since this is unlikely (and is probably bad + * practice), we will assume that they do not exist. + */ + @Override + public Parameter[] optionalParameters() { + return new Parameter[] {}; + } + } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java index 8160da329..7d974bd6f 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java @@ -29,17 +29,25 @@ package org.scijava.ops.matcher; +import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Parameter; import java.lang.reflect.Type; +import java.util.Arrays; import java.util.List; +import java.util.stream.Collectors; import org.scijava.Priority; import org.scijava.ops.OpDependencyMember; import org.scijava.ops.OpInfo; import org.scijava.ops.OpUtils; +import org.scijava.ops.simplify.SimplificationUtils; import org.scijava.ops.simplify.Unsimplifiable; +import org.scijava.ops.util.AnnotationUtils; +import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; import org.scijava.plugin.Plugin; @@ -193,4 +201,34 @@ private static boolean simplifiableFromAnnotation(Class annotationBearer) { public boolean isSimplifiable() { return simplifiable; } + + @Override + public boolean hasOptionalParameters() { + return optionalParameters().length > 0; + + } + + @Override + public Parameter[] optionalParameters() { + Method fMethod; + try { + fMethod = opClass.getMethod(SimplificationUtils.findFMethod(opClass) + .getName(), inputRawTypes()); + } + catch (NoSuchMethodException exc) { + throw new IllegalArgumentException("No Op Method on class " + opClass); + } + return Arrays.stream(fMethod.getParameters()) // + .filter(p -> p.isAnnotationPresent(Optional.class)) // + .toArray(Parameter[]::new); + } + + private Class[] inputRawTypes() { + Type[] params = OpUtils.inputTypes(struct); + Class[] rawParams = new Class[params.length]; + for(int i = 0; i < params.length; i++) { + rawParams[i] = Types.raw(params[i]); + } + return rawParams; + } } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java index aa64ec92a..3cac67679 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java @@ -31,7 +31,9 @@ import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.ArrayList; import java.util.List; @@ -40,6 +42,7 @@ import org.scijava.ops.OpField; import org.scijava.ops.OpInfo; import org.scijava.ops.OpUtils; +import org.scijava.ops.simplify.SimplificationUtils; import org.scijava.ops.simplify.Unsimplifiable; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; @@ -185,4 +188,19 @@ public String toString() { public boolean isSimplifiable() { return simplifiable; } + + /** + * NB annotations are not allowed on Field Ops written as lambdas. Since this + * is really the only way Ops are written as Fields, we are going to blanket + * cover all Field Ops. + */ + @Override + public boolean hasOptionalParameters() { + return false; + } + + @Override + public Parameter[] optionalParameters() { + return new Parameter[] {}; + } } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java index fc832f704..b271e8550 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java @@ -38,6 +38,7 @@ import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; @@ -50,6 +51,7 @@ import org.scijava.ops.OpUtils; import org.scijava.ops.simplify.Unsimplifiable; import org.scijava.ops.util.Adapt; +import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; import org.scijava.param.ValidityProblem; @@ -359,4 +361,16 @@ public AnnotatedElement getAnnotationBearer() { public boolean isSimplifiable() { return simplifiable; } + + @Override + public boolean hasOptionalParameters() { + return optionalParameters().length > 0; + } + + @Override + public Parameter[] optionalParameters() { + return Arrays.stream(method.getParameters()) // + .filter(p -> p.isAnnotationPresent(Optional.class)) // + .toArray(Parameter[]::new); + } } \ No newline at end of file diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplifiedOpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplifiedOpInfo.java index 406c53be0..7f77b865c 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplifiedOpInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplifiedOpInfo.java @@ -1,6 +1,7 @@ package org.scijava.ops.simplify; import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.ArrayList; import java.util.List; @@ -13,6 +14,7 @@ import org.scijava.ops.conversionLoss.LossReporter; import org.scijava.ops.core.Op; import org.scijava.ops.matcher.OpMatchingException; +import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; import org.scijava.struct.Member; @@ -224,6 +226,18 @@ private int compareToSimplifiedInfo(SimplifiedOpInfo that) { return theseMembers.hashCode() - thoseMembers.hashCode(); } + @Override + public boolean hasOptionalParameters() { + return false; + } + /** + * NB Since the simplified Op is autogenerated, we can rest assured that there + * are no {@link Optional} parameters + */ + @Override + public Parameter[] optionalParameters() { + return new Parameter[] {}; + } } From 4ae1c7a872290b1eee24e8229f39ff765fd99de5 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Wed, 26 May 2021 09:47:55 -0500 Subject: [PATCH 03/24] Add ReducedOpInfo: first cut --- .../scijava/ops/matcher/ReducedOpInfo.java | 102 ++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/ReducedOpInfo.java diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/ReducedOpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/ReducedOpInfo.java new file mode 100644 index 000000000..8961a19ed --- /dev/null +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/ReducedOpInfo.java @@ -0,0 +1,102 @@ +package org.scijava.ops.matcher; + +import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Parameter; +import java.lang.reflect.Type; +import java.util.List; + +import org.scijava.ops.OpInfo; +import org.scijava.param.ParameterStructs; +import org.scijava.param.ValidityException; +import org.scijava.struct.Struct; +import org.scijava.struct.StructInstance; + + +public class ReducedOpInfo implements OpInfo { + + private final OpInfo srcInfo; + private final Type reducedOpType; + private final double priority; + + private Struct struct; + private ValidityException validityException; + + public ReducedOpInfo(OpInfo src, Type reducedOpType) { + this.srcInfo = src; + this.reducedOpType = reducedOpType; + + try { + this.struct = ParameterStructs.structOf(srcInfo, reducedOpType); + } + catch (ValidityException e) { + validityException = e; + } + + } + + @Override + public Type opType() { + return reducedOpType; + } + + @Override + public Struct struct() { + return struct; + } + + @Override + public boolean isSimplifiable() { + return true; + } + + @Override + public double priority() { + return srcInfo.priority(); + } + + @Override + public String implementationName() { + // TODO: improve this name + return srcInfo.implementationName() + " as " + reducedOpType.toString(); + } + + @Override + public StructInstance createOpInstance(List dependencies) { + // TODO Auto-generated method stub + return null; + } + + @Override + public boolean isValid() { + return validityException == null; + } + + @Override + public ValidityException getValidityException() { + return validityException; + } + + @Override + public AnnotatedElement getAnnotationBearer() { + return srcInfo.getAnnotationBearer(); + } + + /** + * NB since this {@link OpInfo} has already been reduced, we ignore any + * remaining {@link Optional} parameters + */ + @Override + public boolean hasOptionalParameters() { + return false; + } + + /** + * NB since this {@link OpInfo} has already been reduced, we ignore any + * remaining {@link Optional} parameters + */ + @Override + public Parameter[] optionalParameters() { + return new Parameter[0]; + } + +} From 4f3996756de3fab90b58b94f149733243272bc28 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 1 Jun 2021 13:43:58 -0500 Subject: [PATCH 04/24] Reduction: first cut --- .../src/main/java/module-info.java | 1 + .../src/main/java/org/scijava/ops/OpInfo.java | 4 +- .../main/java/org/scijava/ops/OpUtils.java | 31 +++ .../org/scijava/ops/function/Functions.java | 2 +- .../ops/impl/DefaultOpEnvironment.java | 73 +++++- .../scijava/ops/matcher/OpAdaptationInfo.java | 13 +- .../org/scijava/ops/matcher/OpClassInfo.java | 92 ++++--- .../org/scijava/ops/matcher/OpFieldInfo.java | 44 +++- .../org/scijava/ops/matcher/OpMethodInfo.java | 23 +- .../scijava/ops/reduce/FunctionReducer.java | 38 +++ .../org/scijava/ops/reduce/InfoReducer.java | 12 + .../{matcher => reduce}/ReducedOpInfo.java | 43 ++-- .../scijava/ops/reduce/ReductionUtils.java | 235 ++++++++++++++++++ .../ops/simplify/SimplificationMetadata.java | 17 -- .../ops/simplify/SimplificationUtils.java | 9 +- .../ops/simplify/SimplifiedOpInfo.java | 12 +- .../scijava/ops/OptionalArgumentsTest.java | 26 ++ .../org/scijava/ops/TestOpOptionalArg.java | 24 ++ 18 files changed, 580 insertions(+), 119 deletions(-) create mode 100644 scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/FunctionReducer.java create mode 100644 scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/InfoReducer.java rename scijava/scijava-ops/src/main/java/org/scijava/ops/{matcher => reduce}/ReducedOpInfo.java (56%) create mode 100644 scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java create mode 100644 scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java create mode 100644 scijava/scijava-ops/src/test/java/org/scijava/ops/TestOpOptionalArg.java diff --git a/scijava/scijava-ops/src/main/java/module-info.java b/scijava/scijava-ops/src/main/java/module-info.java index 221e79455..7eb9f4dfb 100644 --- a/scijava/scijava-ops/src/main/java/module-info.java +++ b/scijava/scijava-ops/src/main/java/module-info.java @@ -16,6 +16,7 @@ // -- Open plugins to scijava-common opens org.scijava.ops to org.scijava; opens org.scijava.ops.impl to org.scijava; + opens org.scijava.ops.reduce to org.scijava; // FIXME: This is a file name and is thus unstable requires geantyref; diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/OpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/OpInfo.java index 3a7cf54b4..562d5f856 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/OpInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/OpInfo.java @@ -68,9 +68,7 @@ default OpCandidate createCandidate(OpEnvironment env, Logger log, OpRef ref, Ma AnnotatedElement getAnnotationBearer(); - boolean hasOptionalParameters(); - - Parameter[] optionalParameters(); + boolean isOptional(Member m); @Override default int compareTo(final OpInfo that) { diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/OpUtils.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/OpUtils.java index 32df09bfa..362f38cfa 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/OpUtils.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/OpUtils.java @@ -34,6 +34,7 @@ import java.util.Collections; import java.util.List; import java.util.Objects; +import java.util.Optional; import java.util.stream.Collectors; import org.scijava.ops.matcher.MatchingResult; @@ -136,6 +137,15 @@ public static Type[] inputTypes(Struct struct) { return getTypes(inputs(struct)); } + public static Class[] inputRawTypes(Struct struct) { + Type[] inputTypes = getTypes(inputs(struct)); + Class[] inputRawTypes = new Class[inputTypes.length]; + for (int i = 0; i < inputTypes.length; i++) { + inputRawTypes[i] = Types.raw(inputTypes[i]); + } + return inputRawTypes; + } + public static Member output(OpCandidate candidate) { return candidate.opInfo().output(); } @@ -150,6 +160,10 @@ public static List> outputs(final Struct struct) { .collect(Collectors.toList()); } + public static Type outputType(final Struct candidate) { + return outputs(candidate).get(0).getType(); + } + public static List> outputs(StructInstance op) { return op.members().stream() // .filter(memberInstance -> memberInstance.member().isOutput()) // @@ -435,4 +449,21 @@ public static Class findFirstImplementedFunctionalInterface(final OpRef opRef } return null; } + + /** + * Returns the index of the argument that is both the input and the output. If there is no such argument (i.e. the Op produces a pure output), -1 is returned + * + * @return the index of the mutable argument. + */ + public static int ioArgIndex(final OpInfo info) { + List> inputs = OpUtils.inputs(info.struct()); + Optional> ioArg = inputs.stream().filter(m -> m.isInput() && m.isOutput()).findFirst(); + if(ioArg.isEmpty()) return -1; + Member ioMember = ioArg.get(); + return inputs.indexOf(ioMember); + } + + public static boolean hasPureOutput(final OpInfo info) { + return ioArgIndex(info) == -1; + } } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/function/Functions.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/function/Functions.java index 0b5e0aa49..29b38618b 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/function/Functions.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/function/Functions.java @@ -78,7 +78,7 @@ private Functions() { * @throws NullPointerException If {@code type} is {@code null}. */ public static boolean isFunction(Type type) { - return ALL_FUNCTIONS.containsKey(Types.raw(type)); + return ALL_FUNCTIONS.containsValue(Types.raw(type)); } @SuppressWarnings({ "unchecked" }) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java index f1d244f81..3bd72db18 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java @@ -40,11 +40,14 @@ import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.TreeSet; +import java.util.function.BiConsumer; +import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -72,6 +75,7 @@ import org.scijava.ops.matcher.OpAdaptationInfo; import org.scijava.ops.matcher.OpCandidate; import org.scijava.ops.matcher.OpCandidate.StatusCode; +import org.scijava.ops.reduce.InfoReducer; import org.scijava.ops.matcher.OpClassInfo; import org.scijava.ops.matcher.OpFieldInfo; import org.scijava.ops.matcher.OpMatcher; @@ -89,6 +93,7 @@ import org.scijava.plugin.Parameter; import org.scijava.plugin.PluginInfo; import org.scijava.plugin.PluginService; +import org.scijava.plugin.SciJavaPlugin; import org.scijava.struct.ItemIO; import org.scijava.types.Nil; import org.scijava.types.TypeService; @@ -136,6 +141,11 @@ public class DefaultOpEnvironment extends AbstractContextual implements OpEnviro */ private Map, OpWrapper> wrappers; + /** + * Data structure storing all discoverable {@link InfoReducer}s. + */ + private List infoReducers; + public DefaultOpEnvironment(final Context context) { context.inject(this); matcher = new DefaultOpMatcher(log); @@ -475,6 +485,10 @@ private void initWrappers() { } } + private void initInfoReducers() { + infoReducers = pluginService.createInstancesOfType(InfoReducer.class); + } + /** * Attempts to inject {@link OpDependency} annotated fields of the specified * object by looking for Ops matching the field type and the name specified in @@ -664,6 +678,7 @@ private OpRef inferOpRef(Type type, String name, Map, Type> type private void initOpDirectory() { opDirectory = new HashMap<>(); + initInfoReducers(); // Add regular Ops for (final PluginInfo pluginInfo : pluginService.getPluginsOfType(Op.class)) { @@ -671,6 +686,13 @@ private void initOpDirectory() { final Class opClass = pluginInfo.loadClass(); OpInfo opInfo = new OpClassInfo(opClass); addToOpIndex(opInfo, pluginInfo.getName()); + boolean hasOptional = opInfo.struct().members().parallelStream() // + .filter(m -> opInfo.isOptional(m)) // + .findAny() // + .isPresent(); + if (hasOptional) { + reduceInfo.accept(opInfo, pluginInfo.getName()); + } } catch (InstantiableException exc) { log.error("Can't load class from plugin info: " + pluginInfo.toString(), exc); } @@ -687,18 +709,63 @@ private void initOpDirectory() { instance = field.getDeclaringClass().newInstance(); } OpInfo opInfo = new OpFieldInfo(isStatic ? null : instance, field); - addToOpIndex(opInfo, field.getAnnotation(OpField.class).names()); + String names = field.getAnnotation(OpField.class).names(); + addToOpIndex(opInfo, names); + boolean hasOptional = opInfo.struct().members().parallelStream() // + .filter(m -> opInfo.isOptional(m)) // + .findAny() // + .isPresent(); + if (hasOptional) { + reduceInfo.accept(opInfo, names); + } } final List methods = ClassUtils.getAnnotatedMethods(c, OpMethod.class); for (final Method method: methods) { OpInfo opInfo = new OpMethodInfo(method); - addToOpIndex(opInfo, method.getAnnotation(OpMethod.class).names()); + String names = method.getAnnotation(OpMethod.class).names(); + addToOpIndex(opInfo, names); + boolean hasOptional = opInfo.struct().members().parallelStream() // + .filter(m -> opInfo.isOptional(m)) // + .findAny() // + .isPresent(); + if (hasOptional) { + reduceInfo.accept(opInfo, names); + } } } catch (InstantiableException | InstantiationException | IllegalAccessException exc) { log.error("Can't load class from plugin info: " + pluginInfo.toString(), exc); } } - } + + // TODO: can we +// // Find all Ops able to be reduced +// initInfoReducers(); +// Set reducableInfos = StreamSupport.stream(infos().spliterator(), true) // +// .filter(info -> info.hasOptionalParameters()) // +// .collect(Collectors.toSet()); +// reducableInfos.parallelStream() // +// .forEach(reduceInfo); + } + + private final BiConsumer reduceInfo = (info, names) -> { + // find a InfoReducer capable of reducing info + Optional suitableReducer = infoReducers + .parallelStream().filter(reducer -> reducer.canReduce(info)).findAny(); + if (suitableReducer.isEmpty()) { + log.warn("Cannot reduce " + info + ": No suitable InfoReducer!"); + return; + } + + InfoReducer reducer = suitableReducer.get(); + long numReductions = info.struct().members().parallelStream() // + .filter(m -> info.isOptional(m)) // + .count(); // + // add a ReducedOpInfo for all possible reductions + // TODO: how to find the names? + for (int i = 1; i <= numReductions; i++) { + addToOpIndex(reducer.reduce(info, i), names); + } + }; private void addToOpIndex(final OpInfo opInfo, final String opNames) { String[] parsedOpNames = OpUtils.parseOpNames(opNames); diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpAdaptationInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpAdaptationInfo.java index 55687ff64..c9db86d41 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpAdaptationInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpAdaptationInfo.java @@ -1,7 +1,6 @@ package org.scijava.ops.matcher; import java.lang.reflect.AnnotatedElement; -import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.List; import java.util.function.Function; @@ -12,6 +11,7 @@ import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; +import org.scijava.struct.Member; import org.scijava.struct.Struct; import org.scijava.struct.StructInstance; @@ -107,19 +107,16 @@ public boolean isSimplifiable() { return false; } - @Override - public boolean hasOptionalParameters() { - return false; - } - /** * NB for {@link Optional} annotations to be on the Op, they would have to be * declared within the adapter. Since this is unlikely (and is probably bad * practice), we will assume that they do not exist. */ @Override - public Parameter[] optionalParameters() { - return new Parameter[] {}; + public boolean isOptional(Member m) { + if (!struct.members().contains(m)) throw new IllegalArgumentException( + "Member " + m + " is not a Memeber of OpInfo " + this); + return false; } } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java index 7d974bd6f..89aa5db02 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java @@ -29,7 +29,6 @@ package org.scijava.ops.matcher; -import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; @@ -39,18 +38,20 @@ import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.scijava.Priority; +import org.scijava.ops.FieldOpDependencyMember; import org.scijava.ops.OpDependencyMember; import org.scijava.ops.OpInfo; import org.scijava.ops.OpUtils; import org.scijava.ops.simplify.SimplificationUtils; import org.scijava.ops.simplify.Unsimplifiable; -import org.scijava.ops.util.AnnotationUtils; import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; import org.scijava.plugin.Plugin; +import org.scijava.struct.Member; import org.scijava.struct.Struct; import org.scijava.struct.StructInstance; import org.scijava.types.Types; @@ -67,21 +68,25 @@ public class OpClassInfo implements OpInfo { private Struct struct; private ValidityException validityException; private final double priority; - + private final boolean simplifiable; public OpClassInfo(final Class opClass) { - this(opClass, priorityFromAnnotation(opClass), simplifiableFromAnnotation(opClass)); + this(opClass, priorityFromAnnotation(opClass), simplifiableFromAnnotation( + opClass)); } - public OpClassInfo(final Class opClass, final double priority, final boolean simplifiable) { + public OpClassInfo(final Class opClass, final double priority, + final boolean simplifiable) + { this.opClass = opClass; try { struct = ParameterStructs.structOf(opClass); OpUtils.checkHasSingleOutput(struct); - } catch (ValidityException e) { + } + catch (ValidityException e) { validityException = e; - } + } this.priority = priority; this.simplifiable = simplifiable; } @@ -92,7 +97,7 @@ public OpClassInfo(final Class opClass, final double priority, final boolean public Type opType() { // TODO: Check whether this is correct! return Types.parameterizeRaw(opClass); - //return opClass; + // return opClass; } @Override @@ -154,23 +159,22 @@ public StructInstance createOpInstance(List dependencies) { public ValidityException getValidityException() { return validityException; } - + @Override public boolean isValid() { return validityException == null; } - + @Override public AnnotatedElement getAnnotationBearer() { return opClass; } - + // -- Object methods -- @Override public boolean equals(final Object o) { - if (!(o instanceof OpClassInfo)) - return false; + if (!(o instanceof OpClassInfo)) return false; final OpInfo that = (OpInfo) o; return struct().equals(that.struct()); } @@ -193,7 +197,8 @@ private static double priorityFromAnnotation(Class annotationBearer) { } private static boolean simplifiableFromAnnotation(Class annotationBearer) { - final Unsimplifiable opAnnotation = annotationBearer.getAnnotation(Unsimplifiable.class); + final Unsimplifiable opAnnotation = annotationBearer.getAnnotation( + Unsimplifiable.class); return opAnnotation == null ? true : false; } @@ -202,33 +207,38 @@ public boolean isSimplifiable() { return simplifiable; } + /** + * TODO: this implementation seems hacky, for two reasons. + *
    + *
  1. We are assuming that the Struct's Members and their corresponding + * {@link Parameter}s on the functional Method have the same ordering. There + * is no real guarantee that this is the case.
  2. + *
  3. There doesn't seem to be a good way to reliably determine where the + * {@link Optional} annotations might be. Ideally, they'd be on the functional + * method overriden within the class itself. But sometimes (this happens in + * ImageJ Ops2's geom package) the functional method is overriden within a + * superclass of the class we have, and to reduce code duplication we'd like + * to support that too. It could also be useful to put the annotation directly + * on the method of the Functional Interface (suppose you write a + * {@code BiFunctionWithOptional} interface)
  4. + *
+ */ @Override - public boolean hasOptionalParameters() { - return optionalParameters().length > 0; - - } - - @Override - public Parameter[] optionalParameters() { - Method fMethod; - try { - fMethod = opClass.getMethod(SimplificationUtils.findFMethod(opClass) - .getName(), inputRawTypes()); - } - catch (NoSuchMethodException exc) { - throw new IllegalArgumentException("No Op Method on class " + opClass); - } - return Arrays.stream(fMethod.getParameters()) // - .filter(p -> p.isAnnotationPresent(Optional.class)) // - .toArray(Parameter[]::new); - } - - private Class[] inputRawTypes() { - Type[] params = OpUtils.inputTypes(struct); - Class[] rawParams = new Class[params.length]; - for(int i = 0; i < params.length; i++) { - rawParams[i] = Types.raw(params[i]); - } - return rawParams; + public boolean isOptional(Member m) { + if (!struct.members().contains(m)) throw new IllegalArgumentException( + "Member " + m + " is not a Memeber of OpInfo " + this); + if (m.isOutput()) return false; + if (m instanceof OpDependencyMember) return false; + int inputIndex = OpUtils.inputs(struct).indexOf(m); + // TODO: call this method once? + return parameters().anyMatch(arr -> arr[inputIndex].isAnnotationPresent(Optional.class)); + } + + private Stream parameters() { + Method superFMethod = SimplificationUtils.findFMethod(opClass); + return Arrays.stream(opClass.getMethods()) // + .filter(m -> m.getName().equals(superFMethod.getName())) // + .filter(m -> m.getDeclaringClass() != ParameterStructs.findFunctionalInterface(opClass)) // + .map(m -> m.getParameters()); } } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java index 3cac67679..8e5fd5832 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java @@ -44,9 +44,11 @@ import org.scijava.ops.OpUtils; import org.scijava.ops.simplify.SimplificationUtils; import org.scijava.ops.simplify.Unsimplifiable; +import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; import org.scijava.param.ValidityProblem; +import org.scijava.struct.Member; import org.scijava.struct.Struct; import org.scijava.struct.StructInstance; @@ -189,18 +191,40 @@ public boolean isSimplifiable() { return simplifiable; } - /** - * NB annotations are not allowed on Field Ops written as lambdas. Since this - * is really the only way Ops are written as Fields, we are going to blanket - * cover all Field Ops. - */ @Override - public boolean hasOptionalParameters() { - return false; + public boolean isOptional(Member m) { + if (!struct.members().contains(m)) throw new IllegalArgumentException( + "Member " + m + " is not a Memeber of OpInfo " + this); + if (m.isOutput()) return false; + int inputIndex = OpUtils.inputs(struct).indexOf(m); + // TODO: call this method once? + return parameters()[inputIndex].isAnnotationPresent(Optional.class); } - @Override - public Parameter[] optionalParameters() { - return new Parameter[] {}; + private Parameter[] parameters() { + Class fieldClass; + try { + fieldClass = field.get(instance).getClass(); + } + catch (IllegalArgumentException | IllegalAccessException exc1) { + // TODO Auto-generated catch block + throw new IllegalArgumentException(exc1); + } + Method fMethod = findFunctionalMethod(fieldClass); + return fMethod.getParameters(); + } + + private Method findFunctionalMethod(Class fieldClass) { + Method fMethod = SimplificationUtils.findFMethod(fieldClass); + Class[] inputTypes = fMethod.getParameterTypes(); + if (!fieldClass.isSynthetic()) { + inputTypes = OpUtils.inputRawTypes(struct); + } + try { + return fieldClass.getMethod(fMethod.getName(), inputTypes); + } + catch (NoSuchMethodException exc) { + throw new IllegalArgumentException("No Op Method on class " + fieldClass); + } } } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java index b271e8550..84617634a 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java @@ -38,7 +38,6 @@ import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; @@ -362,15 +361,23 @@ public boolean isSimplifiable() { return simplifiable; } + /** + * TODO: this implementation seems hacky. We are assuming that the Struct's + * Members and their corresponding {@link Parameter}s on the functional Method + * have the same ordering. There is no real guarantee that this is the case. + */ @Override - public boolean hasOptionalParameters() { - return optionalParameters().length > 0; + public boolean isOptional(Member m) { + if (!struct.members().contains(m)) throw new IllegalArgumentException( + "Member " + m + " is not a Memeber of OpInfo " + this); + if (m.isOutput()) return false; + if (m instanceof OpDependencyMember) return false; + int inputIndex = OpUtils.inputs(struct).indexOf(m); + // TODO: call this method once? + return parameters()[inputIndex].isAnnotationPresent(Optional.class); } - @Override - public Parameter[] optionalParameters() { - return Arrays.stream(method.getParameters()) // - .filter(p -> p.isAnnotationPresent(Optional.class)) // - .toArray(Parameter[]::new); + private Parameter[] parameters() { + return method.getParameters(); } } \ No newline at end of file diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/FunctionReducer.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/FunctionReducer.java new file mode 100644 index 000000000..9752eb205 --- /dev/null +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/FunctionReducer.java @@ -0,0 +1,38 @@ +package org.scijava.ops.reduce; + +import java.lang.reflect.Type; + +import org.scijava.ops.OpInfo; +import org.scijava.ops.OpUtils; +import org.scijava.ops.function.Functions; +import org.scijava.param.ParameterStructs; +import org.scijava.plugin.Plugin; +import org.scijava.util.Types; + +@Plugin(type = InfoReducer.class) +public class FunctionReducer implements InfoReducer{ + + @Override + public boolean canReduce(OpInfo info) { + return Functions.isFunction(ParameterStructs.findFunctionalInterface(Types.raw(info.opType()))); + } + + @Override + public ReducedOpInfo reduce(OpInfo info, int numReductions) { + Type opType = info.opType(); + Class rawType = ParameterStructs.findFunctionalInterface(Types.raw(opType)); + Integer originalArity = Functions.ALL_FUNCTIONS.inverse().get(rawType); + Integer reducedArity = originalArity - numReductions; + Class reducedRawType = Functions.ALL_FUNCTIONS.get(reducedArity); + Type[] inputTypes = OpUtils.inputTypes(info.struct()); + Type outputType = OpUtils.outputType(info.struct()); + Type[] newTypes = new Type[reducedArity + 1]; + for(int i = 0; i < reducedArity; i++) { + newTypes[i] = inputTypes[i]; + } + newTypes[newTypes.length - 1] = outputType; + Type reducedOpType = Types.parameterize(reducedRawType, newTypes); + return new ReducedOpInfo(info, reducedOpType, originalArity - reducedArity); + } + +} diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/InfoReducer.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/InfoReducer.java new file mode 100644 index 000000000..52a895c92 --- /dev/null +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/InfoReducer.java @@ -0,0 +1,12 @@ + +package org.scijava.ops.reduce; + +import org.scijava.ops.OpInfo; +import org.scijava.plugin.SciJavaPlugin; + +public interface InfoReducer extends SciJavaPlugin { + + boolean canReduce(OpInfo info); + + ReducedOpInfo reduce(OpInfo info, int numReductions); +} diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/ReducedOpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java similarity index 56% rename from scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/ReducedOpInfo.java rename to scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java index 8961a19ed..35f218f03 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/ReducedOpInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java @@ -1,13 +1,16 @@ -package org.scijava.ops.matcher; +package org.scijava.ops.reduce; import java.lang.reflect.AnnotatedElement; -import java.lang.reflect.Parameter; +import java.lang.reflect.Modifier; import java.lang.reflect.Type; import java.util.List; +import java.util.Objects; import org.scijava.ops.OpInfo; +import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; +import org.scijava.struct.Member; import org.scijava.struct.Struct; import org.scijava.struct.StructInstance; @@ -16,14 +19,15 @@ public class ReducedOpInfo implements OpInfo { private final OpInfo srcInfo; private final Type reducedOpType; - private final double priority; + private final int paramsReduced; private Struct struct; private ValidityException validityException; - public ReducedOpInfo(OpInfo src, Type reducedOpType) { + public ReducedOpInfo(OpInfo src, Type reducedOpType, int paramsReduced) { this.srcInfo = src; this.reducedOpType = reducedOpType; + this.paramsReduced = paramsReduced; try { this.struct = ParameterStructs.structOf(srcInfo, reducedOpType); @@ -31,7 +35,6 @@ public ReducedOpInfo(OpInfo src, Type reducedOpType) { catch (ValidityException e) { validityException = e; } - } @Override @@ -57,13 +60,26 @@ public double priority() { @Override public String implementationName() { // TODO: improve this name - return srcInfo.implementationName() + " as " + reducedOpType.toString(); + return srcInfo.implementationName() + "Reduction" + paramsReduced; } @Override public StructInstance createOpInstance(List dependencies) { - // TODO Auto-generated method stub - return null; + final Object op = srcInfo.createOpInstance(dependencies).object(); + if (!Modifier.isPublic(srcInfo.opType().getClass().getModifiers())) { + throw new IllegalArgumentException("Op " + srcInfo.opType() + + " is not public; only Ops backed by public classes can be reduced!"); + } + try { + Object reducedOp = ReductionUtils.javassistOp(op, this); + return struct().createInstance(reducedOp); + } + catch (Throwable ex) { + throw new IllegalArgumentException( + "Failed to invoke reduction of Op: \n" + srcInfo + + "\nProvided Op dependencies were: " + Objects.toString(dependencies), + ex); + } } @Override @@ -86,17 +102,12 @@ public AnnotatedElement getAnnotationBearer() { * remaining {@link Optional} parameters */ @Override - public boolean hasOptionalParameters() { + public boolean isOptional(Member m) { return false; } - /** - * NB since this {@link OpInfo} has already been reduced, we ignore any - * remaining {@link Optional} parameters - */ - @Override - public Parameter[] optionalParameters() { - return new Parameter[0]; + public OpInfo srcInfo() { + return srcInfo; } } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java new file mode 100644 index 000000000..1bafd6912 --- /dev/null +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java @@ -0,0 +1,235 @@ +package org.scijava.ops.reduce; + +import com.google.common.collect.Streams; + +import java.lang.invoke.MethodHandles; +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Stream; + +import org.scijava.ops.OpUtils; +import org.scijava.ops.simplify.Simplifier; +import org.scijava.param.Optional; +import org.scijava.param.ParameterStructs; +import org.scijava.struct.Member; +import org.scijava.types.Types; + +import javassist.CannotCompileException; +import javassist.ClassPool; +import javassist.CtClass; +import javassist.CtConstructor; +import javassist.CtField; +import javassist.CtMethod; +import javassist.CtNewConstructor; +import javassist.CtNewMethod; +import javassist.Modifier; +import javassist.NotFoundException; + +public class ReductionUtils { + + /** + * Creates a Class given an Op and a set of {@link Simplifier}s. This class: + *
    + *
  • is of the same functional type as the given Op + *
  • has type arguments that are of the simplified form of the type + * arguments of the given Op (these arguments are dictated by the list of + * {@code Simplifier}s. + *
  • + * + * @param originalOp - the Op that will be simplified + * @param reducedInfo - the {@link ReducedOpInfo} containing the information + * required to reduce {@code originalOp}. + * @return a wrapper of {@code originalOp} taking arguments that are then + * mutated to satisfy {@code originalOp}, producing outputs that are + * then mutated to satisfy the desired output of the wrapper. + * @throws Throwable + */ + protected static Object javassistOp(Object originalOp, ReducedOpInfo reducedInfo) throws Throwable { + ClassPool pool = ClassPool.getDefault(); + + // Create wrapper class + String className = formClassName(reducedInfo); + Class c; + try { + c = pool.getClassLoader().loadClass(className); + } + catch (ClassNotFoundException e) { + CtClass cc = generateSimplifiedWrapper(pool, className, reducedInfo); + c = cc.toClass(MethodHandles.lookup()); + } + + // Return Op instance + return c.getDeclaredConstructor(Types.raw(reducedInfo.srcInfo().opType())) + .newInstance(originalOp); + } + + // TODO: consider correctness + private static String formClassName(ReducedOpInfo reducedInfo) { + // package name - required to be this package for the Lookup to work + String packageName = ReductionUtils.class.getPackageName(); + StringBuilder sb = new StringBuilder(packageName + "."); + + // class name + String implementationName = reducedInfo.implementationName(); + String originalName = implementationName.substring(implementationName + .lastIndexOf('.') + 1); // we only want the class name + Stream memberNames = // + Streams.concat(Arrays.stream(OpUtils.inputTypes(reducedInfo.struct())), // + Stream.of(OpUtils.outputType(reducedInfo.struct()))) // + .map(type -> getClassName(Types.raw(type))); + Iterable iterableNames = (Iterable) memberNames::iterator; + String simplifiedParameters = String.join("_", iterableNames); + String className = originalName.concat(simplifiedParameters); + if(className.chars().anyMatch(c -> !Character.isJavaIdentifierPart(c))) + throw new IllegalArgumentException(className + " is not a valid class name!"); + + sb.append(className); + return sb.toString(); + } + + /** + * {@link Class}es of array types return "[]" when + * {@link Class#getSimpleName()} is called. Those characters are invalid in a + * class name, so we exchange them for the suffix "_Arr". + * + * @param clazz - the {@link Class} for which we need a name + * @return - a name that is legal as part of a class name. + */ + private static String getClassName(Class clazz) { + String className = clazz.getSimpleName(); + if(className.chars().allMatch(c -> Character.isJavaIdentifierPart(c))) + return className; + if(clazz.isArray()) + return clazz.getComponentType().getSimpleName() + "_Arr"; + return className; + } + + private static CtClass generateSimplifiedWrapper(ClassPool pool, String className, ReducedOpInfo reducedInfo) throws Throwable + { + CtClass cc = pool.makeClass(className); + // Add implemented interface + CtClass jasOpType = pool.get(Types.raw(reducedInfo.opType()).getName()); + cc.addInterface(jasOpType); + + // Add Op field + CtField opField = createOpField(pool, cc, Types.raw(reducedInfo.srcInfo().opType()), "op"); + cc.addField(opField); + + // Add constructor to take the Simplifiers, as well as the original op. + CtConstructor constructor = CtNewConstructor.make(createConstructor(cc, + reducedInfo), cc); + cc.addConstructor(constructor); + + // add functional interface method + CtMethod functionalMethod = CtNewMethod.make(createFunctionalMethod(reducedInfo), + cc); + cc.addMethod(functionalMethod); + return cc; + } + + private static CtField createOpField(ClassPool pool, CtClass cc, Class opType, String fieldName) + throws NotFoundException, CannotCompileException + { + CtClass fType = pool.get(opType.getName()); + CtField f = new CtField(fType, fieldName, cc); + f.setModifiers(Modifier.PRIVATE + Modifier.FINAL); + return f; + } + + private static String createConstructor(CtClass cc, ReducedOpInfo reducedInfo) { + StringBuilder sb = new StringBuilder(); + // constructor signature + sb.append("public " + cc.getSimpleName() + "("); + // argument - original op + Class opClass = Types.raw(reducedInfo.srcInfo().opType()); + sb.append(" " + opClass.getName() + " op"); + sb.append(") {"); + + sb.append("this.op = op;"); + sb.append("}"); + return sb.toString(); + } + + /** + * Creates the functional method of a reduced Op. This functional method must: + *
      + *
    1. Call the {@code Op} using the required pure inputs, followed by + * {@code null} {@link Optional} pure arguments, followed by the i/o + * argument (iff it exists). + *
    + * NB The Javassist compiler + * does + * not fully support generics, so we must ensure that the types are raw. + * At compile time, the raw types are equivalent to the generic types, so this + * should not pose any issues. + * + * @param info - the {@link ReducedOpInfo} containing the + * information needed to write the method. + * @return a {@link String} that can be used by + * {@link CtMethod#make(String, CtClass)} to generate the functional + * method of the simplified Op + */ + private static String createFunctionalMethod(ReducedOpInfo info) { + StringBuilder sb = new StringBuilder(); + + // determine the name of the functional method + Method m = ParameterStructs.singularAbstractMethod(Types.raw(info.opType())); + // determine the name of the output: + String opOutput = "out"; + + //-- signature -- // + sb.append(generateSignature(m)); + + //-- body --// + + // processing + sb.append(" {"); + if (OpUtils.hasPureOutput(info)) { + sb.append(OpUtils.outputType(info.struct()).getTypeName() + " " + opOutput + " = "); + } + sb.append("op." + m.getName() + "("); + int i; + List> totalArguments = OpUtils.inputs(info.srcInfo().struct()); + int reducedArg = 0; + for (i = 0; i < totalArguments.size(); i++) { + if (info.srcInfo().isOptional(totalArguments.get(i))) { + sb.append(" null"); + } + else { + sb.append(" in" + reducedArg++); + } + if (i + 1 < totalArguments.size()) sb.append(","); + } + + sb.append(");"); + + // if pure output, return it + if (OpUtils.hasPureOutput(info)) { + sb.append("return out;"); + } + sb.append("}"); + return sb.toString(); + } + + private static String generateSignature(Method m) { + StringBuilder sb = new StringBuilder(); + String methodName = m.getName(); + + // method modifiers + boolean isVoid = m.getReturnType() == void.class; + sb.append("public " + (isVoid ? "void" : "Object") + " " + methodName + + "("); + + int inputs = m.getParameterCount(); + for (int i = 0; i < inputs; i++) { + sb.append(" Object in" + i); + if (i < inputs - 1) sb.append(","); + } + + sb.append(" )"); + + return sb.toString(); + } + +} diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplificationMetadata.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplificationMetadata.java index 17a82a3d7..bf4d845d8 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplificationMetadata.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplificationMetadata.java @@ -226,23 +226,6 @@ public Object[] constructorArgs(Object op) { return args.toArray(); } - /** - * Returns the index of the argument that is both the input and the output. If there is no such argument (i.e. the Op produces a pure output), -1 is returned - * - * @return the index of the mutable argument. - */ - public int ioArgIndex() { - List> inputs = OpUtils.inputs(info.struct()); - Optional> ioArg = inputs.stream().filter(m -> m.isInput() && m.isOutput()).findFirst(); - if(ioArg.isEmpty()) return -1; - Member ioMember = ioArg.get(); - return inputs.indexOf(ioMember); - } - - public boolean pureOutput() { - return ioArgIndex() == -1; - } - public OpInfo info() { return info; } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplificationUtils.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplificationUtils.java index fd0f28201..fdb778f2b 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplificationUtils.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplificationUtils.java @@ -20,6 +20,7 @@ import org.scijava.ops.OpEnvironment; import org.scijava.ops.OpInfo; +import org.scijava.ops.OpUtils; import org.scijava.ops.function.Computers; import org.scijava.ops.matcher.MatchingUtils; import org.scijava.ops.matcher.OpRef; @@ -479,7 +480,7 @@ private static String createFunctionalMethod(SimplificationMetadata metadata) { Method m = ParameterStructs.singularAbstractMethod(metadata.opType()); // determine the name of the output: String opOutput = ""; - int ioIndex = metadata.ioArgIndex(); + int ioIndex = OpUtils.ioArgIndex(metadata.info()); if(ioIndex == -1) { opOutput = "originalOut"; } @@ -497,7 +498,7 @@ private static String createFunctionalMethod(SimplificationMetadata metadata) { sb.append(fMethodPreprocessing(metadata)); // processing - if (metadata.pureOutput()) { + if (OpUtils.hasPureOutput(metadata.info())) { sb.append(metadata.originalOutput().getTypeName() + " " + opOutput + " = "); } sb.append("op." + m.getName() + "("); @@ -511,7 +512,7 @@ private static String createFunctionalMethod(SimplificationMetadata metadata) { sb.append(fMethodPostprocessing(metadata, opOutput)); // if pure output, return it - if (metadata.pureOutput()) { + if (OpUtils.hasPureOutput(metadata.info())) { sb.append("return out;"); } sb.append("}"); @@ -556,7 +557,7 @@ private static String fMethodPostprocessing(SimplificationMetadata metadata, Str // call copy op iff it exists if(metadata.hasCopyOp()) { - int ioIndex = metadata.ioArgIndex(); + int ioIndex = OpUtils.ioArgIndex(metadata.info()); Type ioType = metadata.originalInputs()[ioIndex]; String originalIOArg = "in" + ioIndex; sb.append("copyOp.compute((" + focused.getTypeName() + ") out, (" + ioType.getTypeName() + ") " + originalIOArg + ");"); diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplifiedOpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplifiedOpInfo.java index 7f77b865c..8219be323 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplifiedOpInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplifiedOpInfo.java @@ -1,7 +1,6 @@ package org.scijava.ops.simplify; import java.lang.reflect.AnnotatedElement; -import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.ArrayList; import java.util.List; @@ -226,18 +225,15 @@ private int compareToSimplifiedInfo(SimplifiedOpInfo that) { return theseMembers.hashCode() - thoseMembers.hashCode(); } - @Override - public boolean hasOptionalParameters() { - return false; - } - /** * NB Since the simplified Op is autogenerated, we can rest assured that there * are no {@link Optional} parameters */ @Override - public Parameter[] optionalParameters() { - return new Parameter[] {}; + public boolean isOptional(Member m) { + if (!struct.members().contains(m)) throw new IllegalArgumentException( + "Member " + m + " is not a Memeber of OpInfo " + this); + return false; } } diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java new file mode 100644 index 000000000..45b15303e --- /dev/null +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java @@ -0,0 +1,26 @@ +package org.scijava.ops; + +import org.junit.Assert; +import org.junit.Test; +import org.scijava.ops.core.OpCollection; +import org.scijava.plugin.Plugin; + +@Plugin(type = OpCollection.class) +public class OptionalArgumentsTest extends AbstractTestEnvironment{ + + @Test + public void testClassWithOptional() { + Double sum = ops.env().op("test.optionalAdd").input(2.0, 5.0).outType(Double.class).apply(); + Double expected = 7.0; + Assert.assertEquals(expected, sum); + } + + @Test + public void testClassWithoutOptional() { + Double sum = ops.env().op("test.optionalAdd").input(2.0).outType(Double.class).apply(); + Double expected = 2.0; + Assert.assertEquals(expected, sum); + + } + +} diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/TestOpOptionalArg.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/TestOpOptionalArg.java new file mode 100644 index 000000000..4d78b303b --- /dev/null +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/TestOpOptionalArg.java @@ -0,0 +1,24 @@ + +package org.scijava.ops; + +import java.util.function.BiFunction; + +import org.scijava.ops.core.Op; +import org.scijava.param.Optional; +import org.scijava.param.Parameter; +import org.scijava.plugin.Plugin; +import org.scijava.struct.ItemIO; + +@Plugin(type = Op.class, name = "test.optionalAdd") +@Parameter(key = "in1") +@Parameter(key = "in2") +@Parameter(key = "out", itemIO = ItemIO.OUTPUT) +public class TestOpOptionalArg implements BiFunction { + + @Override + public Double apply(Double t, @Optional Double u) { + if (u == null) return t; + return t + u; + } + +} From c7fd79033d9ac32f765497ba28897b3458e874c3 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 1 Jun 2021 15:01:00 -0500 Subject: [PATCH 05/24] Stream methods resembling the functional method We want to leave open the possibility for @Optional annotations to be declared on the interface as well as on the Op implementation. This means we have to check multiple Methods for the Optional annotation. This could be a source of bugs, but for now we check any method with the same name as the functional method, having the same number of parameters. --- .../org/scijava/ops/matcher/OpClassInfo.java | 2 +- .../org/scijava/ops/matcher/OpFieldInfo.java | 27 +++++++------------ .../org/scijava/ops/matcher/OpMethodInfo.java | 17 +++++++++--- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java index 89aa5db02..3124fcc11 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java @@ -238,7 +238,7 @@ private Stream parameters() { Method superFMethod = SimplificationUtils.findFMethod(opClass); return Arrays.stream(opClass.getMethods()) // .filter(m -> m.getName().equals(superFMethod.getName())) // - .filter(m -> m.getDeclaringClass() != ParameterStructs.findFunctionalInterface(opClass)) // + .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // .map(m -> m.getParameters()); } } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java index 8e5fd5832..dc318d7f9 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java @@ -36,7 +36,9 @@ import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.stream.Stream; import org.scijava.Priority; import org.scijava.ops.OpField; @@ -198,10 +200,11 @@ public boolean isOptional(Member m) { if (m.isOutput()) return false; int inputIndex = OpUtils.inputs(struct).indexOf(m); // TODO: call this method once? - return parameters()[inputIndex].isAnnotationPresent(Optional.class); + return parameters().anyMatch(arr -> arr[inputIndex].isAnnotationPresent(Optional.class)); } - private Parameter[] parameters() { + + private Stream parameters() { Class fieldClass; try { fieldClass = field.get(instance).getClass(); @@ -210,21 +213,11 @@ private Parameter[] parameters() { // TODO Auto-generated catch block throw new IllegalArgumentException(exc1); } - Method fMethod = findFunctionalMethod(fieldClass); - return fMethod.getParameters(); + Method superFMethod = SimplificationUtils.findFMethod(fieldClass); + return Arrays.stream(fieldClass.getMethods()) // + .filter(m -> m.getName().equals(superFMethod.getName())) // + .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // + .map(m -> m.getParameters()); } - private Method findFunctionalMethod(Class fieldClass) { - Method fMethod = SimplificationUtils.findFMethod(fieldClass); - Class[] inputTypes = fMethod.getParameterTypes(); - if (!fieldClass.isSynthetic()) { - inputTypes = OpUtils.inputRawTypes(struct); - } - try { - return fieldClass.getMethod(fMethod.getName(), inputTypes); - } - catch (NoSuchMethodException exc) { - throw new IllegalArgumentException("No Op Method on class " + fieldClass); - } - } } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java index 84617634a..50d751715 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java @@ -30,6 +30,8 @@ package org.scijava.ops.matcher; +import com.google.common.collect.Streams; + import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.reflect.AnnotatedElement; @@ -38,9 +40,11 @@ import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.scijava.Priority; import org.scijava.ops.OpDependency; @@ -48,6 +52,7 @@ import org.scijava.ops.OpInfo; import org.scijava.ops.OpMethod; import org.scijava.ops.OpUtils; +import org.scijava.ops.simplify.SimplificationUtils; import org.scijava.ops.simplify.Unsimplifiable; import org.scijava.ops.util.Adapt; import org.scijava.param.Optional; @@ -374,10 +379,16 @@ public boolean isOptional(Member m) { if (m instanceof OpDependencyMember) return false; int inputIndex = OpUtils.inputs(struct).indexOf(m); // TODO: call this method once? - return parameters()[inputIndex].isAnnotationPresent(Optional.class); + return parameters().anyMatch(arr -> arr[inputIndex].isAnnotationPresent(Optional.class)); } - private Parameter[] parameters() { - return method.getParameters(); + private Stream parameters() { + Method superFMethod = SimplificationUtils.findFMethod(Types.raw(opType)); + List methods = new ArrayList<>(Arrays.asList(opType.getClass().getMethods())); + methods.add(method); + return methods.parallelStream() // + .filter(m -> m.getName().equals(superFMethod.getName())) // + .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // + .map(m -> m.getParameters()); } } \ No newline at end of file From 6af7c3611cb8baba46b14505a9def0aec4421f25 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Tue, 1 Jun 2021 16:20:42 -0500 Subject: [PATCH 06/24] Test Reductions for Op written as Field Field reductions will be very rare, since you cannot annotate lambda parameters, but it is not too difficult to add support for Field Ops that are anonymous classes --- .../scijava/ops/reduce/ComputerReducer.java | 38 +++++++++++ .../org/scijava/ops/reduce/ReducedOpInfo.java | 4 ++ .../scijava/ops/reduce/ReductionUtils.java | 20 ++++-- .../scijava/ops/OptionalArgumentsTest.java | 68 ++++++++++++++++++- .../org/scijava/ops/TestOpOptionalArg.java | 15 ++-- 5 files changed, 133 insertions(+), 12 deletions(-) create mode 100644 scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ComputerReducer.java diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ComputerReducer.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ComputerReducer.java new file mode 100644 index 000000000..2570dfbed --- /dev/null +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ComputerReducer.java @@ -0,0 +1,38 @@ +package org.scijava.ops.reduce; + +import java.lang.reflect.Type; + +import org.scijava.ops.OpInfo; +import org.scijava.ops.OpUtils; +import org.scijava.ops.function.Computers; +import org.scijava.param.ParameterStructs; +import org.scijava.plugin.Plugin; +import org.scijava.util.Types; + +@Plugin(type = InfoReducer.class) +public class ComputerReducer implements InfoReducer{ + + @Override + public boolean canReduce(OpInfo info) { + return Computers.isComputer(ParameterStructs.findFunctionalInterface(Types.raw(info.opType()))); + } + + @Override + public ReducedOpInfo reduce(OpInfo info, int numReductions) { + Type opType = info.opType(); + Class rawType = ParameterStructs.findFunctionalInterface(Types.raw(opType)); + Integer originalArity = Computers.ALL_COMPUTERS.get(rawType); + Integer reducedArity = originalArity - numReductions; + Class reducedRawType = Computers.ALL_COMPUTERS.inverse().get(reducedArity); + Type[] inputTypes = OpUtils.inputTypes(info.struct()); + Type outputType = OpUtils.outputType(info.struct()); + Type[] newTypes = new Type[reducedArity + 1]; + for(int i = 0; i < reducedArity; i++) { + newTypes[i] = inputTypes[i]; + } + newTypes[newTypes.length - 1] = outputType; + Type reducedOpType = Types.parameterize(reducedRawType, newTypes); + return new ReducedOpInfo(info, reducedOpType, originalArity - reducedArity); + } + +} diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java index 35f218f03..d8830b238 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java @@ -110,4 +110,8 @@ public OpInfo srcInfo() { return srcInfo; } + public int paramsReduced() { + return paramsReduced; + } + } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java index 1bafd6912..f25f252e0 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java @@ -191,13 +191,25 @@ private static String createFunctionalMethod(ReducedOpInfo info) { sb.append("op." + m.getName() + "("); int i; List> totalArguments = OpUtils.inputs(info.srcInfo().struct()); + int totalArgs = OpUtils.inputs(info.srcInfo().struct()).size(); + long totalOptionals = OpUtils.inputs(info.srcInfo().struct()) + .parallelStream().filter(member -> info.srcInfo().isOptional(member)) + .count(); + long neededOptionals = totalOptionals - info.paramsReduced(); int reducedArg = 0; - for (i = 0; i < totalArguments.size(); i++) { - if (info.srcInfo().isOptional(totalArguments.get(i))) { - sb.append(" null"); + int optionals = 0; + for (i = 0; i < totalArgs; i++) { + // NB due to our optionality paradigm (if there are n optional parameters, + // they must be the last n), we just need to pass null for the last n + // arguments + if (!info.srcInfo().isOptional(totalArguments.get(i))) { + sb.append(" in" + reducedArg++); + } else if (optionals < neededOptionals) { + sb.append(" in" + reducedArg++); + optionals++; } else { - sb.append(" in" + reducedArg++); + sb.append(" null"); } if (i + 1 < totalArguments.size()) sb.append(","); } diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java index 45b15303e..8ff760d30 100644 --- a/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java @@ -1,26 +1,90 @@ package org.scijava.ops; +import java.util.Arrays; + import org.junit.Assert; import org.junit.Test; import org.scijava.ops.core.OpCollection; +import org.scijava.ops.function.Computers; +import org.scijava.param.Container; +import org.scijava.param.Optional; import org.scijava.plugin.Plugin; @Plugin(type = OpCollection.class) public class OptionalArgumentsTest extends AbstractTestEnvironment{ @Test - public void testClassWithOptional() { + public void testClassWithTwoOptionals() { + Double sum = ops.env().op("test.optionalAdd").input(2.0, 5.0, 7.0).outType(Double.class).apply(); + Double expected = 14.0; + Assert.assertEquals(expected, sum); + } + + @Test + public void testClassWithOneOptional() { Double sum = ops.env().op("test.optionalAdd").input(2.0, 5.0).outType(Double.class).apply(); Double expected = 7.0; Assert.assertEquals(expected, sum); } @Test - public void testClassWithoutOptional() { + public void testClassWithoutOptionals() { Double sum = ops.env().op("test.optionalAdd").input(2.0).outType(Double.class).apply(); Double expected = 2.0; Assert.assertEquals(expected, sum); + } + + @OpField(names = "test.optionalMultiply") + public final Computers.Arity3 optionalField = + new Computers.Arity3<>() + { + + @Override + public void compute(Double[] in1, @Optional Double[] in2, + @Optional Double[] in3, Double[] out) + { + if (in2 == null) { + in2 = new Double[in1.length]; + Arrays.fill(in2, 1.); + } + if (in3 == null) { + in3 = new Double[in1.length]; + Arrays.fill(in3, 1.); + } + for (int i = 0; i < in1.length; i++) { + out[i] = in1[i] * in2[i] * in3[i]; + } + } + }; + @Test + public void testFieldWithTwoOptionals() { + Double[] d1 = {2.0}; + Double[] d2 = {5.0}; + Double[] d3 = {7.0}; + Double[] o = {50.0}; + ops.env().op("test.optionalMultiply").input(d1, d2, d3).output(o).compute(); + Double expected = 70.0; + Assert.assertEquals(expected, o[0]); + } + + @Test + public void testFieldWithOneOptional() { + Double[] d1 = {2.0}; + Double[] d2 = {5.0}; + Double[] o = {50.0}; + ops.env().op("test.optionalMultiply").input(d1, d2).output(o).compute(); + Double expected = 10.0; + Assert.assertEquals(expected, o[0]); + } + + @Test + public void testFieldWithoutOptionals() { + Double[] d1 = {2.0}; + Double[] o = {50.0}; + ops.env().op("test.optionalMultiply").input(d1).output(o).compute(); + Double expected = 2.0; + Assert.assertEquals(expected, o[0]); } } diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/TestOpOptionalArg.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/TestOpOptionalArg.java index 4d78b303b..6f4467a4b 100644 --- a/scijava/scijava-ops/src/test/java/org/scijava/ops/TestOpOptionalArg.java +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/TestOpOptionalArg.java @@ -1,24 +1,27 @@ package org.scijava.ops; -import java.util.function.BiFunction; - import org.scijava.ops.core.Op; import org.scijava.param.Optional; import org.scijava.param.Parameter; import org.scijava.plugin.Plugin; import org.scijava.struct.ItemIO; +import org.scijava.ops.function.Functions; @Plugin(type = Op.class, name = "test.optionalAdd") @Parameter(key = "in1") @Parameter(key = "in2") +@Parameter(key = "in3") @Parameter(key = "out", itemIO = ItemIO.OUTPUT) -public class TestOpOptionalArg implements BiFunction { +public class TestOpOptionalArg implements + Functions.Arity3 +{ @Override - public Double apply(Double t, @Optional Double u) { - if (u == null) return t; - return t + u; + public Double apply(Double t, @Optional Double u, @Optional Double v) { + if (u == null) u = 0.; + if (v == null) v = 0.; + return t + u + v; } } From 4cc519e0dc0b04c37cfe5a14a0641468297fc4a2 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Wed, 2 Jun 2021 10:33:18 -0500 Subject: [PATCH 07/24] Clean DefaultOpEnvironment streams --- .../ops/impl/DefaultOpEnvironment.java | 27 +++++-------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java index 3bd72db18..b8e5da4b9 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java @@ -686,10 +686,8 @@ private void initOpDirectory() { final Class opClass = pluginInfo.loadClass(); OpInfo opInfo = new OpClassInfo(opClass); addToOpIndex(opInfo, pluginInfo.getName()); - boolean hasOptional = opInfo.struct().members().parallelStream() // - .filter(m -> opInfo.isOptional(m)) // - .findAny() // - .isPresent(); + boolean hasOptional = OpUtils.inputs(opInfo.struct()).parallelStream() // + .anyMatch(m -> opInfo.isOptional(m)); // if (hasOptional) { reduceInfo.accept(opInfo, pluginInfo.getName()); } @@ -711,10 +709,8 @@ private void initOpDirectory() { OpInfo opInfo = new OpFieldInfo(isStatic ? null : instance, field); String names = field.getAnnotation(OpField.class).names(); addToOpIndex(opInfo, names); - boolean hasOptional = opInfo.struct().members().parallelStream() // - .filter(m -> opInfo.isOptional(m)) // - .findAny() // - .isPresent(); + boolean hasOptional = OpUtils.inputs(opInfo.struct()).parallelStream() // + .anyMatch(m -> opInfo.isOptional(m)); // if (hasOptional) { reduceInfo.accept(opInfo, names); } @@ -724,10 +720,8 @@ private void initOpDirectory() { OpInfo opInfo = new OpMethodInfo(method); String names = method.getAnnotation(OpMethod.class).names(); addToOpIndex(opInfo, names); - boolean hasOptional = opInfo.struct().members().parallelStream() // - .filter(m -> opInfo.isOptional(m)) // - .findAny() // - .isPresent(); + boolean hasOptional = OpUtils.inputs(opInfo.struct()).parallelStream() // + .anyMatch(m -> opInfo.isOptional(m)); // if (hasOptional) { reduceInfo.accept(opInfo, names); } @@ -736,15 +730,6 @@ private void initOpDirectory() { log.error("Can't load class from plugin info: " + pluginInfo.toString(), exc); } } - - // TODO: can we -// // Find all Ops able to be reduced -// initInfoReducers(); -// Set reducableInfos = StreamSupport.stream(infos().spliterator(), true) // -// .filter(info -> info.hasOptionalParameters()) // -// .collect(Collectors.toSet()); -// reducableInfos.parallelStream() // -// .forEach(reduceInfo); } private final BiConsumer reduceInfo = (info, names) -> { From 458d9a2e8eaab9e27a5f25a85b05b45ad0d0addf Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Wed, 2 Jun 2021 12:09:30 -0500 Subject: [PATCH 08/24] Test OpMethods --- .../src/main/java/module-info.java | 1 + .../org/scijava/ops/matcher/OpMethodInfo.java | 5 +- .../org/scijava/ops/reduce/ReducedOpInfo.java | 8 +-- .../scijava/ops/reduce/ReductionUtils.java | 62 +++++++++++++++---- .../scijava/ops/OptionalArgumentsTest.java | 29 +++++++++ 5 files changed, 87 insertions(+), 18 deletions(-) diff --git a/imagej/imagej-ops2/src/main/java/module-info.java b/imagej/imagej-ops2/src/main/java/module-info.java index 248aa56e7..5c9599563 100644 --- a/imagej/imagej-ops2/src/main/java/module-info.java +++ b/imagej/imagej-ops2/src/main/java/module-info.java @@ -1,6 +1,7 @@ module net.imagej.ops2 { // -- Open plugins to scijava-common + opens net.imagej.ops2; opens net.imagej.ops2.coloc to org.scijava, org.scijava.ops; opens net.imagej.ops2.coloc.icq to org.scijava, org.scijava.ops; opens net.imagej.ops2.coloc.kendallTau to org.scijava, org.scijava.ops; diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java index 50d751715..f17563a7c 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java @@ -377,9 +377,12 @@ public boolean isOptional(Member m) { "Member " + m + " is not a Memeber of OpInfo " + this); if (m.isOutput()) return false; if (m instanceof OpDependencyMember) return false; + // TODO: this likely will break down if OpDependencies int inputIndex = OpUtils.inputs(struct).indexOf(m); // TODO: call this method once? - return parameters().anyMatch(arr -> arr[inputIndex].isAnnotationPresent(Optional.class)); + boolean optionalOnMethod = method.getParameters()[inputIndex].isAnnotationPresent(Optional.class); + boolean optionalOnIFace = parameters().anyMatch(arr -> arr[inputIndex].isAnnotationPresent(Optional.class)); + return optionalOnMethod || optionalOnIFace; } private Stream parameters() { diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java index d8830b238..dde338213 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java @@ -66,10 +66,10 @@ public String implementationName() { @Override public StructInstance createOpInstance(List dependencies) { final Object op = srcInfo.createOpInstance(dependencies).object(); - if (!Modifier.isPublic(srcInfo.opType().getClass().getModifiers())) { - throw new IllegalArgumentException("Op " + srcInfo.opType() + - " is not public; only Ops backed by public classes can be reduced!"); - } +// if (!Modifier.isPublic(srcInfo.opType().getClass().getModifiers())) { +// throw new IllegalArgumentException("Op " + srcInfo.opType() + +// " is not public; only Ops backed by public classes can be reduced!"); +// } try { Object reducedOp = ReductionUtils.javassistOp(op, this); return struct().createInstance(reducedOp); diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java index f25f252e0..ae4c92e91 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java @@ -64,30 +64,66 @@ protected static Object javassistOp(Object originalOp, ReducedOpInfo reducedInfo .newInstance(originalOp); } - // TODO: consider correctness + /** + * A valid class name must be unique. + * @param reducedInfo + * @return the class name + */ private static String formClassName(ReducedOpInfo reducedInfo) { - // package name - required to be this package for the Lookup to work - String packageName = ReductionUtils.class.getPackageName(); + // -- package name -- + // NB required to be this package for the Lookup to work + String packageName = getPackageName(); StringBuilder sb = new StringBuilder(packageName + "."); - // class name - String implementationName = reducedInfo.implementationName(); - String originalName = implementationName.substring(implementationName - .lastIndexOf('.') + 1); // we only want the class name - Stream memberNames = // - Streams.concat(Arrays.stream(OpUtils.inputTypes(reducedInfo.struct())), // - Stream.of(OpUtils.outputType(reducedInfo.struct()))) // - .map(type -> getClassName(Types.raw(type))); - Iterable iterableNames = (Iterable) memberNames::iterator; - String simplifiedParameters = String.join("_", iterableNames); + // -- class name -- + // Start with the class of the implementation + String originalName = className(reducedInfo); + // Add the input members for uniqueness + String simplifiedParameters = memberNames(reducedInfo); + + // -- ensure the name is valid -- String className = originalName.concat(simplifiedParameters); if(className.chars().anyMatch(c -> !Character.isJavaIdentifierPart(c))) throw new IllegalArgumentException(className + " is not a valid class name!"); + // -- full name is package + class -- sb.append(className); return sb.toString(); } + private static String getPackageName() { + return ReductionUtils.class.getPackageName(); + } + + private static String className(ReducedOpInfo reducedInfo) { + String implName = reducedInfo.implementationName(); + int parenIndex = implName.indexOf('('); + int classStart; + // no paren - structure is package.class + if (parenIndex == -1) { + classStart = implName.lastIndexOf('.') + 1; + } + // paren - structure is packge.class.method(params) + else { + int methodStart = implName.substring(0, parenIndex).lastIndexOf('.'); + classStart = implName.substring(0, methodStart).lastIndexOf('.') + 1; + } + + String originalName = implName.substring(classStart); // we only want the class name + // replace non-valid identifiers with underscore (the underscore is arbitrary) + return originalName.replaceAll("[^A-Z^a-z0-9$_]", "_"); + } + + private static String memberNames(ReducedOpInfo reducedInfo) { + Stream memberNames = // + Streams.concat(Arrays.stream(OpUtils.inputTypes(reducedInfo.struct())), // + Stream.of(OpUtils.outputType(reducedInfo.struct()))) // + .map(type -> getClassName(Types.raw(type))); + Iterable iterableNames = (Iterable) memberNames::iterator; + String simplifiedParameters = String.join("_", iterableNames); + return simplifiedParameters; + } + /** * {@link Class}es of array types return "[]" when * {@link Class#getSimpleName()} is called. Those characters are invalid in a diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java index 8ff760d30..5d03b9a38 100644 --- a/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java @@ -6,6 +6,7 @@ import org.junit.Test; import org.scijava.ops.core.OpCollection; import org.scijava.ops.function.Computers; +import org.scijava.ops.function.Functions; import org.scijava.param.Container; import org.scijava.param.Optional; import org.scijava.plugin.Plugin; @@ -87,4 +88,32 @@ public void testFieldWithoutOptionals() { Assert.assertEquals(expected, o[0]); } + @OpMethod(names = "test.optionalConcatenate", type = Functions.Arity3.class) + public static String optionalMethod(String in1, @Optional String in2, @Optional String in3) { + if (in2 == null) in2 = ""; + if (in3 == null) in3 = ""; + return in1.concat(in2).concat(in3); + } + + @Test + public void testMethodWithTwoOptionals() { + String out = ops.env().op("test.optionalConcatenate").input("a", "b", "c").outType(String.class).apply(); + String expected = "abc"; + Assert.assertEquals(expected, out); + } + + @Test + public void testMethodWithOneOptional() { + String out = ops.env().op("test.optionalConcatenate").input("a", "b").outType(String.class).apply(); + String expected = "ab"; + Assert.assertEquals(expected, out); + } + + @Test + public void testMethodWithoutOptionals() { + String out = ops.env().op("test.optionalConcatenate").input("a").outType(String.class).apply(); + String expected = "a"; + Assert.assertEquals(expected, out); + } + } From 6c1352ce98cb4bccf1633b00cedc8ffd2c415ae0 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Wed, 2 Jun 2021 12:50:14 -0500 Subject: [PATCH 09/24] Move reduction tests to reduction package --- .../org/scijava/ops/{ => reduce}/OptionalArgumentsTest.java | 6 ++++-- .../org/scijava/ops/{ => reduce}/TestOpOptionalArg.java | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) rename scijava/scijava-ops/src/test/java/org/scijava/ops/{ => reduce}/OptionalArgumentsTest.java (92%) rename scijava/scijava-ops/src/test/java/org/scijava/ops/{ => reduce}/TestOpOptionalArg.java (91%) diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsTest.java similarity index 92% rename from scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java rename to scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsTest.java index 5d03b9a38..b36f99568 100644 --- a/scijava/scijava-ops/src/test/java/org/scijava/ops/OptionalArgumentsTest.java +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsTest.java @@ -1,13 +1,15 @@ -package org.scijava.ops; +package org.scijava.ops.reduce; import java.util.Arrays; import org.junit.Assert; import org.junit.Test; +import org.scijava.ops.AbstractTestEnvironment; +import org.scijava.ops.OpField; +import org.scijava.ops.OpMethod; import org.scijava.ops.core.OpCollection; import org.scijava.ops.function.Computers; import org.scijava.ops.function.Functions; -import org.scijava.param.Container; import org.scijava.param.Optional; import org.scijava.plugin.Plugin; diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/TestOpOptionalArg.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/TestOpOptionalArg.java similarity index 91% rename from scijava/scijava-ops/src/test/java/org/scijava/ops/TestOpOptionalArg.java rename to scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/TestOpOptionalArg.java index 6f4467a4b..cbeccc578 100644 --- a/scijava/scijava-ops/src/test/java/org/scijava/ops/TestOpOptionalArg.java +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/TestOpOptionalArg.java @@ -1,5 +1,5 @@ -package org.scijava.ops; +package org.scijava.ops.reduce; import org.scijava.ops.core.Op; import org.scijava.param.Optional; From 124964266a2f548b92bad1048fe55ecf3b76d442 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Wed, 2 Jun 2021 14:22:19 -0500 Subject: [PATCH 10/24] Test reduction extensibility --- .../org/scijava/ops/matcher/OpMethodInfo.java | 2 +- .../ops/reduce/BiFunctionWithOptional.java | 14 ++++++++ .../reduce/BiFunctionWithOptionalReducer.java | 36 +++++++++++++++++++ .../OptionalArgumentsFromIFaceTest.java | 33 +++++++++++++++++ 4 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/BiFunctionWithOptional.java create mode 100644 scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/BiFunctionWithOptionalReducer.java create mode 100644 scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsFromIFaceTest.java diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java index f17563a7c..52cf4c815 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java @@ -388,7 +388,7 @@ public boolean isOptional(Member m) { private Stream parameters() { Method superFMethod = SimplificationUtils.findFMethod(Types.raw(opType)); List methods = new ArrayList<>(Arrays.asList(opType.getClass().getMethods())); - methods.add(method); + methods.add(superFMethod); return methods.parallelStream() // .filter(m -> m.getName().equals(superFMethod.getName())) // .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/BiFunctionWithOptional.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/BiFunctionWithOptional.java new file mode 100644 index 000000000..926fa3807 --- /dev/null +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/BiFunctionWithOptional.java @@ -0,0 +1,14 @@ + +package org.scijava.ops.reduce; + +import org.scijava.ops.function.Functions; +import org.scijava.param.Optional; + +@FunctionalInterface +public interface BiFunctionWithOptional extends + Functions.Arity3 +{ + + @Override + public O apply(I1 in1, I2 in2, @Optional I3 in3); +} diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/BiFunctionWithOptionalReducer.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/BiFunctionWithOptionalReducer.java new file mode 100644 index 000000000..a00271044 --- /dev/null +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/BiFunctionWithOptionalReducer.java @@ -0,0 +1,36 @@ +package org.scijava.ops.reduce; + +import java.lang.reflect.Type; + +import org.scijava.ops.OpInfo; +import org.scijava.ops.OpUtils; +import org.scijava.ops.function.Functions; +import org.scijava.param.ParameterStructs; +import org.scijava.plugin.Plugin; +import org.scijava.types.Types; + +@Plugin(type = InfoReducer.class) +public class BiFunctionWithOptionalReducer implements InfoReducer { + + @Override + public boolean canReduce(OpInfo info) { + return ParameterStructs.findFunctionalInterface(Types.raw(info.opType())) == BiFunctionWithOptional.class; + } + + @Override + public ReducedOpInfo reduce(OpInfo info, int numReductions) { + if (numReductions != 1) throw new UnsupportedOperationException(); + int reducedArity = 3 - numReductions; + Class reducedRawType = Functions.ALL_FUNCTIONS.get(reducedArity); + Type[] inputTypes = OpUtils.inputTypes(info.struct()); + Type outputType = OpUtils.outputType(info.struct()); + Type[] newTypes = new Type[reducedArity + 1]; + for(int i = 0; i < reducedArity; i++) { + newTypes[i] = inputTypes[i]; + } + newTypes[newTypes.length - 1] = outputType; + Type reducedOpType = Types.parameterize(reducedRawType, newTypes); + return new ReducedOpInfo(info, reducedOpType, numReductions); + } + +} diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsFromIFaceTest.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsFromIFaceTest.java new file mode 100644 index 000000000..8eb8b77e2 --- /dev/null +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsFromIFaceTest.java @@ -0,0 +1,33 @@ +package org.scijava.ops.reduce; + +import org.junit.Assert; +import org.junit.Test; +import org.scijava.ops.AbstractTestEnvironment; +import org.scijava.ops.OpMethod; +import org.scijava.ops.core.OpCollection; +import org.scijava.plugin.Plugin; + +@Plugin(type = OpCollection.class) +public class OptionalArgumentsFromIFaceTest extends AbstractTestEnvironment{ + + @OpMethod(names = "test.optionalSubtract", type = BiFunctionWithOptional.class) + public static Double foo(Double i1, Double i2, Double i3) { + if (i3 == null) i3 = 0.; + return i1 - i2 - i3; + } + + @Test + public void testMethodWithOneOptional() { + Double o = ops.env().op("test.optionalSubtract").input(2., 5., 7.).outType(Double.class).apply(); + Double expected = -10.0; + Assert.assertEquals(expected, o); + } + + @Test + public void testMethodWithoutOptionals() { + Double o = ops.env().op("test.optionalSubtract").input(2., 5.).outType(Double.class).apply(); + Double expected = -3.0; + Assert.assertEquals(expected, o); + } + +} From 83e6009df97524e60bcb3764f5bc7b6fe5036bf8 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Wed, 2 Jun 2021 15:41:50 -0500 Subject: [PATCH 11/24] First cut: prerecord parameter optionality --- .../org/scijava/ops/matcher/OpMethodInfo.java | 58 ++++++++++++++++--- 1 file changed, 50 insertions(+), 8 deletions(-) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java index 52cf4c815..e7fd07ffc 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java @@ -30,8 +30,6 @@ package org.scijava.ops.matcher; -import com.google.common.collect.Streams; - import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.reflect.AnnotatedElement; @@ -82,6 +80,7 @@ public class OpMethodInfo implements OpInfo { private final Method method; private Type opType; private Struct struct; + private final boolean[] paramOptionality; private final ValidityException validityException; private final boolean simplifiable; @@ -117,6 +116,8 @@ public OpMethodInfo(final Method method) { catch (final ValidityException e) { problems.addAll(e.problems()); } + + paramOptionality = getParameterOptionality(this.method, Types.raw(opType)); validityException = problems.isEmpty() ? null : new ValidityException( problems); } @@ -379,19 +380,60 @@ public boolean isOptional(Member m) { if (m instanceof OpDependencyMember) return false; // TODO: this likely will break down if OpDependencies int inputIndex = OpUtils.inputs(struct).indexOf(m); - // TODO: call this method once? - boolean optionalOnMethod = method.getParameters()[inputIndex].isAnnotationPresent(Optional.class); - boolean optionalOnIFace = parameters().anyMatch(arr -> arr[inputIndex].isAnnotationPresent(Optional.class)); - return optionalOnMethod || optionalOnIFace; + return paramOptionality[inputIndex]; + } + + private static boolean[] getParameterOptionality(Method m, Class opType) { + int[] paramIndex = mapFunctionalParamsToIndices(m.getParameters()); + boolean[] arr = new boolean[m.getParameterCount()]; + // check parameters on m + boolean[] mOptionals = hasOptionalAnnotation(m.getParameters()); + // check parameters on methods from opType + boolean[][] optionalOnIFace = parameters(opType); + + for (boolean[] a : optionalOnIFace) { + for(int i = 0; i < arr.length; i++) { + int index = paramIndex[i]; + if (index == -1) continue; + arr[i] |= a[index]; + } + } + for(int i = 0; i < arr.length; i++) { + arr[i] |= mOptionals[i]; + } + return arr; } - private Stream parameters() { + private static int[] mapFunctionalParamsToIndices(Parameter[] parameters) { + int[] paramNo = new int[parameters.length]; + int paramIndex = 0; + for(int i = 0; i < parameters.length; i++) { + if (parameters[i].isAnnotationPresent(OpDependency.class)) { + paramNo[i] = -1; + } + else { + paramNo[i] = paramIndex++; + } + } + return paramNo; + } + + private static boolean[][] parameters(Class opType) { Method superFMethod = SimplificationUtils.findFMethod(Types.raw(opType)); List methods = new ArrayList<>(Arrays.asList(opType.getClass().getMethods())); methods.add(superFMethod); return methods.parallelStream() // .filter(m -> m.getName().equals(superFMethod.getName())) // .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // - .map(m -> m.getParameters()); + .map(m -> hasOptionalAnnotation(m.getParameters())) // + .toArray(boolean[][]::new); + } + + private static boolean[] hasOptionalAnnotation(Parameter[] params) { + boolean[] b = new boolean[params.length]; + for(int i = 0; i < params.length; i++) { + b[i] = params[i].isAnnotationPresent(Optional.class); + } + return b; } } \ No newline at end of file From a570f161365198c2af2f08e43250591fdd99bd5b Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Thu, 3 Jun 2021 10:14:59 -0500 Subject: [PATCH 12/24] Explain reasoning for one method annotated --- .../main/java/org/scijava/param/Optional.java | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/param/Optional.java b/scijava/scijava-ops/src/main/java/org/scijava/param/Optional.java index 160afdb92..4c55191fa 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/param/Optional.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/param/Optional.java @@ -9,6 +9,34 @@ /** * Annotation to mark a parameter as optional: Ops with optional parameters * should be callable with or without their Optional arguments + *

    + * This annotation should only be specified on one of the signatures of + * the method (only on the {@link FunctionalInterface}'s method, or only on the + * Op implementation, etc) for purposes of simplicity and readability. Writing + * + *

    + * public interface BiFunctionWithOptional<I1, I2, I3, O> extends
    + * 	Functions.Arity3<>
    + * {
    + * 
    + * 	public O apply(I1 in1, I2 in2, @Optional I3 in3);
    + * }
    + * 
    + * + * and then writing an implementation + * + *
    + * public class Impl implements BiFunctionWithOptional<Double, Double, Double, Double> {
    + * 	public Double apply(Double in1, @Optional Double in2, Double in3) {
    + * 	...
    + * 	}
    + * }
    + * 
    + * + * is confusing and hard to read. Which parameters are optional in this case? Is + * it obvious that {@code in3} is optional just by looking at {@code Impl}? For + * this reason, it should be enforced that the annotation is only on one of the + * method signatures. * * @author Gabriel Selzer */ From 8a4bdb24ab31f81a9613d1e84f8e358dbc441b14 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Thu, 3 Jun 2021 10:19:59 -0500 Subject: [PATCH 13/24] Clean OpMethodInfo's Optionality decision making This was really confusing, now it is less so :) --- .../org/scijava/ops/matcher/OpMethodInfo.java | 98 +++++++++++++------ 1 file changed, 69 insertions(+), 29 deletions(-) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java index e7fd07ffc..6f237c33a 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java @@ -41,8 +41,8 @@ import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.function.Function; import java.util.stream.Collectors; -import java.util.stream.Stream; import org.scijava.Priority; import org.scijava.ops.OpDependency; @@ -80,7 +80,7 @@ public class OpMethodInfo implements OpInfo { private final Method method; private Type opType; private Struct struct; - private final boolean[] paramOptionality; + private Boolean[] paramOptionality; private final ValidityException validityException; private final boolean simplifiable; @@ -117,7 +117,10 @@ public OpMethodInfo(final Method method) { problems.addAll(e.problems()); } - paramOptionality = getParameterOptionality(this.method, Types.raw(opType)); + // determine parameter optionality + paramOptionality = getParameterOptionality(this.method, Types.raw(opType), + struct, problems); + validityException = problems.isEmpty() ? null : new ValidityException( problems); } @@ -383,27 +386,64 @@ public boolean isOptional(Member m) { return paramOptionality[inputIndex]; } - private static boolean[] getParameterOptionality(Method m, Class opType) { + private static Boolean[] getParameterOptionality(Method m, Class opType, + Struct struct, List problems) + { + boolean opMethodHasOptionals = hasOptionalAnntotations.apply(m); + List fMethodsWithOptionals = fMethodsWithOptional(opType); + // the number of parameters we need to determine + int opParams = OpUtils.inputs(struct).size(); + + // Ensure only the Op method OR ONE of its op type's functional methods have + // Optionals + if (opMethodHasOptionals && !fMethodsWithOptionals.isEmpty()) { + problems.add(new ValidityProblem( + "Both the OpMethod and its op type have optional parameters!")); + return generateAllRequiredArray.apply(opParams); + } + if (fMethodsWithOptionals.size() > 1) { + problems.add(new ValidityProblem( + "Multiple methods from the op type have optional parameters!")); + return generateAllRequiredArray.apply(opParams); + } + + // return the optionality of each parameter of the Op + if (opMethodHasOptionals) return getOpMethodOptionals(m, opParams); + if (fMethodsWithOptionals.size() > 0) return findParameterOptionality.apply( + fMethodsWithOptionals.get(0)); + return generateAllRequiredArray.apply(opParams); + } + + private static Function generateAllRequiredArray = + num -> { + Boolean[] arr = new Boolean[num]; + Arrays.fill(arr, false); + return arr; + }; + + private static Boolean[] getOpMethodOptionals(Method m, int opParams) { int[] paramIndex = mapFunctionalParamsToIndices(m.getParameters()); - boolean[] arr = new boolean[m.getParameterCount()]; + Boolean[] arr = generateAllRequiredArray.apply(opParams); // check parameters on m - boolean[] mOptionals = hasOptionalAnnotation(m.getParameters()); - // check parameters on methods from opType - boolean[][] optionalOnIFace = parameters(opType); - - for (boolean[] a : optionalOnIFace) { - for(int i = 0; i < arr.length; i++) { - int index = paramIndex[i]; - if (index == -1) continue; - arr[i] |= a[index]; - } - } + Boolean[] mOptionals = findParameterOptionality.apply(m); for(int i = 0; i < arr.length; i++) { - arr[i] |= mOptionals[i]; + int index = paramIndex[i]; + if (index == -1) continue; + arr[i] |= mOptionals[index]; } return arr; } + /** + * Since {@link OpMethod}s can have an {@link OpDependency} (or multiple) as + * parameters, we need to determine which parameter indices correspond to the + * inputs of the Op. + * + * @param parameters the list of {@link Parameter}s of the {@link OpMethod} + * @return an array of ints where the value at index {@code i} denotes the + * position of the parameter in the Op's signature. Values of + * {@code -1} designate an {@link OpDependency} at that position. + */ private static int[] mapFunctionalParamsToIndices(Parameter[] parameters) { int[] paramNo = new int[parameters.length]; int paramIndex = 0; @@ -418,22 +458,22 @@ private static int[] mapFunctionalParamsToIndices(Parameter[] parameters) { return paramNo; } - private static boolean[][] parameters(Class opType) { + private static List fMethodsWithOptional(Class opType) { Method superFMethod = SimplificationUtils.findFMethod(Types.raw(opType)); List methods = new ArrayList<>(Arrays.asList(opType.getClass().getMethods())); methods.add(superFMethod); return methods.parallelStream() // - .filter(m -> m.getName().equals(superFMethod.getName())) // - .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // - .map(m -> hasOptionalAnnotation(m.getParameters())) // - .toArray(boolean[][]::new); + .filter(m -> m.getName().equals(superFMethod.getName())) // + .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // + .filter(m -> hasOptionalAnntotations.apply(m)) // + .collect(Collectors.toList()); } - private static boolean[] hasOptionalAnnotation(Parameter[] params) { - boolean[] b = new boolean[params.length]; - for(int i = 0; i < params.length; i++) { - b[i] = params[i].isAnnotationPresent(Optional.class); - } - return b; - } + private static Function findParameterOptionality = + m -> Arrays.stream(m.getParameters()).map(p -> p.isAnnotationPresent( + Optional.class)).toArray(Boolean[]::new); + + private static Function hasOptionalAnntotations = + m -> Arrays.stream(m.getParameters()).anyMatch(p -> p.isAnnotationPresent( + Optional.class)); } \ No newline at end of file From f2316decfe793343355392628412c00853f62ff0 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Thu, 3 Jun 2021 13:24:19 -0500 Subject: [PATCH 14/24] Test Class inheriting optional arguments In writing this test I found that getMethods works differntly than I expected. I knew that there were two instances of the functional method that were being returned by Class.getMethods(). I did not realize that both were from the class (I had thought that one came from the implemented functional interface). It turns out that both come from the class, so we need to do some extra introspection on the functional interface --- .../org/scijava/ops/matcher/OpClassInfo.java | 66 +++++++++++++++++-- .../OptionalArgumentsFromIFaceTest.java | 19 ++++++ .../ops/reduce/TestOpOptionalFromIFace.java | 21 ++++++ 3 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/TestOpOptionalFromIFace.java diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java index 3124fcc11..994287fce 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java @@ -35,13 +35,13 @@ import java.lang.reflect.Method; import java.lang.reflect.Parameter; import java.lang.reflect.Type; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.function.Function; import java.util.stream.Collectors; -import java.util.stream.Stream; import org.scijava.Priority; -import org.scijava.ops.FieldOpDependencyMember; import org.scijava.ops.OpDependencyMember; import org.scijava.ops.OpInfo; import org.scijava.ops.OpUtils; @@ -50,6 +50,7 @@ import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; +import org.scijava.param.ValidityProblem; import org.scijava.plugin.Plugin; import org.scijava.struct.Member; import org.scijava.struct.Struct; @@ -69,6 +70,7 @@ public class OpClassInfo implements OpInfo { private ValidityException validityException; private final double priority; + private final Boolean[] paramOptionality; private final boolean simplifiable; public OpClassInfo(final Class opClass) { @@ -79,16 +81,26 @@ public OpClassInfo(final Class opClass) { public OpClassInfo(final Class opClass, final double priority, final boolean simplifiable) { + final List problems = new ArrayList<>(); + this.opClass = opClass; try { struct = ParameterStructs.structOf(opClass); OpUtils.checkHasSingleOutput(struct); } catch (ValidityException e) { - validityException = e; + problems.addAll(e.problems()); } this.priority = priority; this.simplifiable = simplifiable; + + // determine parameter optionality + paramOptionality = getParameterOptionality(opClass, + struct, problems); + + + validityException = problems.isEmpty() ? null : new ValidityException( + problems); } // -- OpInfo methods -- @@ -231,14 +243,56 @@ public boolean isOptional(Member m) { if (m instanceof OpDependencyMember) return false; int inputIndex = OpUtils.inputs(struct).indexOf(m); // TODO: call this method once? - return parameters().anyMatch(arr -> arr[inputIndex].isAnnotationPresent(Optional.class)); + return paramOptionality[inputIndex]; + } + + private static Boolean[] getParameterOptionality(Class opType, + Struct struct, List problems) + { + List fMethodsWithOptionals = fMethodsWithOptional(opType); + Class fIface = ParameterStructs.findFunctionalInterface(opType); + List fIfaceMethodsWithOptionals = fMethodsWithOptional(fIface); + // the number of parameters we need to determine + int opParams = OpUtils.inputs(struct).size(); + + if (fMethodsWithOptionals.isEmpty() && fIfaceMethodsWithOptionals.isEmpty()) { + return generateAllRequiredArray.apply(opParams); + } + if (!fMethodsWithOptionals.isEmpty() && !fIfaceMethodsWithOptionals.isEmpty()) { + problems.add(new ValidityProblem( + "Multiple methods from the op type have optional parameters!")); + return generateAllRequiredArray.apply(opParams); + } + if (fMethodsWithOptionals.isEmpty()) { + return findParameterOptionality.apply(fIfaceMethodsWithOptionals.get(0)); + } + if (fIfaceMethodsWithOptionals.isEmpty()) { + return findParameterOptionality.apply(fMethodsWithOptionals.get(0)); + } + return generateAllRequiredArray.apply(opParams); } - private Stream parameters() { + private static Function generateAllRequiredArray = + num -> { + Boolean[] arr = new Boolean[num]; + Arrays.fill(arr, false); + return arr; + }; + + private static List fMethodsWithOptional(Class opClass) { Method superFMethod = SimplificationUtils.findFMethod(opClass); return Arrays.stream(opClass.getMethods()) // .filter(m -> m.getName().equals(superFMethod.getName())) // .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // - .map(m -> m.getParameters()); + .filter(m -> hasOptionalAnnotations.apply(m)) // + .collect(Collectors.toList()); } + + private static Function findParameterOptionality = + m -> Arrays.stream(m.getParameters()).map(p -> p.isAnnotationPresent( + Optional.class)).toArray(Boolean[]::new); + + private static Function hasOptionalAnnotations = + m -> Arrays.stream(m.getParameters()).anyMatch(p -> p.isAnnotationPresent( + Optional.class)); } diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsFromIFaceTest.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsFromIFaceTest.java index 8eb8b77e2..83d41ae6f 100644 --- a/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsFromIFaceTest.java +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsFromIFaceTest.java @@ -30,4 +30,23 @@ public void testMethodWithoutOptionals() { Assert.assertEquals(expected, o); } + @Test + public void testClassWithOptionals() { + Boolean in1 = true; + Boolean in2 = false; + Boolean in3 = false; + Boolean o = ops.env().op("test.optionalOr").input(in1, in2, in3).outType(Boolean.class).apply(); + Boolean expected = true; + Assert.assertEquals(expected, o); + } + + @Test + public void testClassWithoutOptionals() { + Boolean in1 = true; + Boolean in2 = false; + Boolean o = ops.env().op("test.optionalOr").input(in1, in2).outType(Boolean.class).apply(); + Boolean expected = true; + Assert.assertEquals(expected, o); + } + } diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/TestOpOptionalFromIFace.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/TestOpOptionalFromIFace.java new file mode 100644 index 000000000..be439a081 --- /dev/null +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/TestOpOptionalFromIFace.java @@ -0,0 +1,21 @@ +package org.scijava.ops.reduce; + +import org.scijava.struct.ItemIO; +import org.scijava.ops.core.Op; +import org.scijava.param.Parameter; +import org.scijava.plugin.Plugin; + +@Plugin(type = Op.class, name = "test.optionalOr") +@Parameter(key = "in1") +@Parameter(key = "in2") +@Parameter(key = "in3") +@Parameter(key = "out", itemIO = ItemIO.OUTPUT) +public class TestOpOptionalFromIFace implements BiFunctionWithOptional{ + + @Override + public Boolean apply(Boolean in1, Boolean in2, Boolean in3) { + if (in3 == null) in3 = false; + return in1 | in2 | in3; + } + +} From 7ad47c22bf48bbc7a17200a54be92b51b9dcc2dc Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Thu, 3 Jun 2021 13:38:16 -0500 Subject: [PATCH 15/24] Test OpField with functional iface optional args --- .../org/scijava/ops/matcher/OpFieldInfo.java | 70 ++++++++++++++++--- .../OptionalArgumentsFromIFaceTest.java | 27 +++++++ 2 files changed, 86 insertions(+), 11 deletions(-) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java index dc318d7f9..c0966a92d 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java @@ -33,12 +33,12 @@ import java.lang.reflect.Field; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.stream.Stream; +import java.util.function.Function; +import java.util.stream.Collectors; import org.scijava.Priority; import org.scijava.ops.OpField; @@ -67,6 +67,7 @@ public class OpFieldInfo implements OpInfo { private Struct struct; private ValidityException validityException; + private final Boolean[] paramOptionality; private final boolean simplifiable; public OpFieldInfo(final Object instance, final Field field) { @@ -107,6 +108,10 @@ public OpFieldInfo(final Object instance, final Field field) { // we cannot simplify the Op iff it has the Unsimplifiable annotation. simplifiable = field.getAnnotation(Unsimplifiable.class) == null; + + // determine parameter optionality + paramOptionality = getParameterOptionality(instance, field, + struct, problems); } // -- OpInfo methods -- @@ -200,24 +205,67 @@ public boolean isOptional(Member m) { if (m.isOutput()) return false; int inputIndex = OpUtils.inputs(struct).indexOf(m); // TODO: call this method once? - return parameters().anyMatch(arr -> arr[inputIndex].isAnnotationPresent(Optional.class)); + return paramOptionality[inputIndex]; } + private static Boolean[] getParameterOptionality(Object instance, Field field, + Struct struct, List problems) + { + // the number of parameters we need to determine + int opParams = OpUtils.inputs(struct).size(); - private Stream parameters() { Class fieldClass; try { fieldClass = field.get(instance).getClass(); } - catch (IllegalArgumentException | IllegalAccessException exc1) { + catch (IllegalArgumentException | IllegalAccessException exc) { // TODO Auto-generated catch block - throw new IllegalArgumentException(exc1); + problems.add(new ValidityProblem(exc)); + return generateAllRequiredArray.apply(opParams); + } + List fMethodsWithOptionals = fMethodsWithOptional(fieldClass); + Class fIface = ParameterStructs.findFunctionalInterface(fieldClass); + List fIfaceMethodsWithOptionals = fMethodsWithOptional(fIface); + + if (fMethodsWithOptionals.isEmpty() && fIfaceMethodsWithOptionals.isEmpty()) { + return generateAllRequiredArray.apply(opParams); + } + if (!fMethodsWithOptionals.isEmpty() && !fIfaceMethodsWithOptionals.isEmpty()) { + problems.add(new ValidityProblem( + "Multiple methods from the op type have optional parameters!")); + return generateAllRequiredArray.apply(opParams); + } + if (fMethodsWithOptionals.isEmpty()) { + return findParameterOptionality.apply(fIfaceMethodsWithOptionals.get(0)); } - Method superFMethod = SimplificationUtils.findFMethod(fieldClass); - return Arrays.stream(fieldClass.getMethods()) // - .filter(m -> m.getName().equals(superFMethod.getName())) // - .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // - .map(m -> m.getParameters()); + if (fIfaceMethodsWithOptionals.isEmpty()) { + return findParameterOptionality.apply(fMethodsWithOptionals.get(0)); + } + return generateAllRequiredArray.apply(opParams); + } + + private static Function generateAllRequiredArray = + num -> { + Boolean[] arr = new Boolean[num]; + Arrays.fill(arr, false); + return arr; + }; + + private static List fMethodsWithOptional(Class opClass) { + Method superFMethod = SimplificationUtils.findFMethod(opClass); + return Arrays.stream(opClass.getMethods()) // + .filter(m -> m.getName().equals(superFMethod.getName())) // + .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // + .filter(m -> hasOptionalAnnotations.apply(m)) // + .collect(Collectors.toList()); } + private static Function findParameterOptionality = + m -> Arrays.stream(m.getParameters()).map(p -> p.isAnnotationPresent( + Optional.class)).toArray(Boolean[]::new); + + private static Function hasOptionalAnnotations = + m -> Arrays.stream(m.getParameters()).anyMatch(p -> p.isAnnotationPresent( + Optional.class)); + } diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsFromIFaceTest.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsFromIFaceTest.java index 83d41ae6f..05d305554 100644 --- a/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsFromIFaceTest.java +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/OptionalArgumentsFromIFaceTest.java @@ -3,6 +3,7 @@ import org.junit.Assert; import org.junit.Test; import org.scijava.ops.AbstractTestEnvironment; +import org.scijava.ops.OpField; import org.scijava.ops.OpMethod; import org.scijava.ops.core.OpCollection; import org.scijava.plugin.Plugin; @@ -30,6 +31,32 @@ public void testMethodWithoutOptionals() { Assert.assertEquals(expected, o); } + @OpField(names = "test.optionalAnd", params = "in1, in2, in3") + public final BiFunctionWithOptional bar = + (in1, in2, in3) -> { + if (in3 == null) in3 = true; + return in1 & in2 & in3; + }; + + @Test + public void testFieldWithOptionals() { + Boolean in1 = true; + Boolean in2 = true; + Boolean in3 = false; + Boolean o = ops.env().op("test.optionalAnd").input(in1, in2, in3).outType(Boolean.class).apply(); + Boolean expected = false; + Assert.assertEquals(expected, o); + } + + @Test + public void testFieldWithoutOptionals() { + Boolean in1 = true; + Boolean in2 = true; + Boolean o = ops.env().op("test.optionalAnd").input(in1, in2).outType(Boolean.class).apply(); + Boolean expected = true; + Assert.assertEquals(expected, o); + } + @Test public void testClassWithOptionals() { Boolean in1 = true; From a38cab467f2d10e2f6191bea18650397661e441e Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Thu, 3 Jun 2021 14:03:17 -0500 Subject: [PATCH 16/24] Extract common logic to ReductionUtils --- .../org/scijava/ops/matcher/OpClassInfo.java | 43 ++++------------- .../org/scijava/ops/matcher/OpFieldInfo.java | 46 ++++--------------- .../org/scijava/ops/matcher/OpMethodInfo.java | 46 ++++--------------- .../scijava/ops/reduce/ReductionUtils.java | 28 +++++++++++ 4 files changed, 54 insertions(+), 109 deletions(-) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java index 994287fce..80936da9f 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java @@ -36,16 +36,13 @@ import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; -import java.util.function.Function; -import java.util.stream.Collectors; import org.scijava.Priority; import org.scijava.ops.OpDependencyMember; import org.scijava.ops.OpInfo; import org.scijava.ops.OpUtils; -import org.scijava.ops.simplify.SimplificationUtils; +import org.scijava.ops.reduce.ReductionUtils; import org.scijava.ops.simplify.Unsimplifiable; import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; @@ -249,50 +246,26 @@ public boolean isOptional(Member m) { private static Boolean[] getParameterOptionality(Class opType, Struct struct, List problems) { - List fMethodsWithOptionals = fMethodsWithOptional(opType); + List fMethodsWithOptionals = ReductionUtils.fMethodsWithOptional(opType); Class fIface = ParameterStructs.findFunctionalInterface(opType); - List fIfaceMethodsWithOptionals = fMethodsWithOptional(fIface); + List fIfaceMethodsWithOptionals = ReductionUtils.fMethodsWithOptional(fIface); // the number of parameters we need to determine int opParams = OpUtils.inputs(struct).size(); - if (fMethodsWithOptionals.isEmpty() && fIfaceMethodsWithOptionals.isEmpty()) { - return generateAllRequiredArray.apply(opParams); + return ReductionUtils.generateAllRequiredArray(opParams); } if (!fMethodsWithOptionals.isEmpty() && !fIfaceMethodsWithOptionals.isEmpty()) { problems.add(new ValidityProblem( "Multiple methods from the op type have optional parameters!")); - return generateAllRequiredArray.apply(opParams); + return ReductionUtils.generateAllRequiredArray(opParams); } if (fMethodsWithOptionals.isEmpty()) { - return findParameterOptionality.apply(fIfaceMethodsWithOptionals.get(0)); + return ReductionUtils.findParameterOptionality(fIfaceMethodsWithOptionals.get(0)); } if (fIfaceMethodsWithOptionals.isEmpty()) { - return findParameterOptionality.apply(fMethodsWithOptionals.get(0)); + return ReductionUtils.findParameterOptionality(fMethodsWithOptionals.get(0)); } - return generateAllRequiredArray.apply(opParams); + return ReductionUtils.generateAllRequiredArray(opParams); } - private static Function generateAllRequiredArray = - num -> { - Boolean[] arr = new Boolean[num]; - Arrays.fill(arr, false); - return arr; - }; - - private static List fMethodsWithOptional(Class opClass) { - Method superFMethod = SimplificationUtils.findFMethod(opClass); - return Arrays.stream(opClass.getMethods()) // - .filter(m -> m.getName().equals(superFMethod.getName())) // - .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // - .filter(m -> hasOptionalAnnotations.apply(m)) // - .collect(Collectors.toList()); - } - - private static Function findParameterOptionality = - m -> Arrays.stream(m.getParameters()).map(p -> p.isAnnotationPresent( - Optional.class)).toArray(Boolean[]::new); - - private static Function hasOptionalAnnotations = - m -> Arrays.stream(m.getParameters()).anyMatch(p -> p.isAnnotationPresent( - Optional.class)); } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java index c0966a92d..801817f90 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java @@ -35,18 +35,14 @@ import java.lang.reflect.Modifier; import java.lang.reflect.Type; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; -import java.util.function.Function; -import java.util.stream.Collectors; import org.scijava.Priority; import org.scijava.ops.OpField; import org.scijava.ops.OpInfo; import org.scijava.ops.OpUtils; -import org.scijava.ops.simplify.SimplificationUtils; +import org.scijava.ops.reduce.ReductionUtils; import org.scijava.ops.simplify.Unsimplifiable; -import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; import org.scijava.param.ValidityProblem; @@ -221,51 +217,27 @@ private static Boolean[] getParameterOptionality(Object instance, Field field, catch (IllegalArgumentException | IllegalAccessException exc) { // TODO Auto-generated catch block problems.add(new ValidityProblem(exc)); - return generateAllRequiredArray.apply(opParams); + return ReductionUtils.generateAllRequiredArray(opParams); } - List fMethodsWithOptionals = fMethodsWithOptional(fieldClass); + List fMethodsWithOptionals = ReductionUtils.fMethodsWithOptional(fieldClass); Class fIface = ParameterStructs.findFunctionalInterface(fieldClass); - List fIfaceMethodsWithOptionals = fMethodsWithOptional(fIface); + List fIfaceMethodsWithOptionals = ReductionUtils.fMethodsWithOptional(fIface); if (fMethodsWithOptionals.isEmpty() && fIfaceMethodsWithOptionals.isEmpty()) { - return generateAllRequiredArray.apply(opParams); + return ReductionUtils.generateAllRequiredArray(opParams); } if (!fMethodsWithOptionals.isEmpty() && !fIfaceMethodsWithOptionals.isEmpty()) { problems.add(new ValidityProblem( "Multiple methods from the op type have optional parameters!")); - return generateAllRequiredArray.apply(opParams); + return ReductionUtils.generateAllRequiredArray(opParams); } if (fMethodsWithOptionals.isEmpty()) { - return findParameterOptionality.apply(fIfaceMethodsWithOptionals.get(0)); + return ReductionUtils.findParameterOptionality(fIfaceMethodsWithOptionals.get(0)); } if (fIfaceMethodsWithOptionals.isEmpty()) { - return findParameterOptionality.apply(fMethodsWithOptionals.get(0)); + return ReductionUtils.findParameterOptionality(fMethodsWithOptionals.get(0)); } - return generateAllRequiredArray.apply(opParams); + return ReductionUtils.generateAllRequiredArray(opParams); } - private static Function generateAllRequiredArray = - num -> { - Boolean[] arr = new Boolean[num]; - Arrays.fill(arr, false); - return arr; - }; - - private static List fMethodsWithOptional(Class opClass) { - Method superFMethod = SimplificationUtils.findFMethod(opClass); - return Arrays.stream(opClass.getMethods()) // - .filter(m -> m.getName().equals(superFMethod.getName())) // - .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // - .filter(m -> hasOptionalAnnotations.apply(m)) // - .collect(Collectors.toList()); - } - - private static Function findParameterOptionality = - m -> Arrays.stream(m.getParameters()).map(p -> p.isAnnotationPresent( - Optional.class)).toArray(Boolean[]::new); - - private static Function hasOptionalAnnotations = - m -> Arrays.stream(m.getParameters()).anyMatch(p -> p.isAnnotationPresent( - Optional.class)); - } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java index 6f237c33a..ce0c85b25 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java @@ -38,10 +38,8 @@ import java.lang.reflect.Parameter; import java.lang.reflect.Type; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Objects; -import java.util.function.Function; import java.util.stream.Collectors; import org.scijava.Priority; @@ -50,10 +48,9 @@ import org.scijava.ops.OpInfo; import org.scijava.ops.OpMethod; import org.scijava.ops.OpUtils; -import org.scijava.ops.simplify.SimplificationUtils; +import org.scijava.ops.reduce.ReductionUtils; import org.scijava.ops.simplify.Unsimplifiable; import org.scijava.ops.util.Adapt; -import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; import org.scijava.param.ValidityProblem; @@ -389,8 +386,8 @@ public boolean isOptional(Member m) { private static Boolean[] getParameterOptionality(Method m, Class opType, Struct struct, List problems) { - boolean opMethodHasOptionals = hasOptionalAnntotations.apply(m); - List fMethodsWithOptionals = fMethodsWithOptional(opType); + boolean opMethodHasOptionals = ReductionUtils.hasOptionalAnnotations(m); + List fMethodsWithOptionals = ReductionUtils.fMethodsWithOptional(opType); // the number of parameters we need to determine int opParams = OpUtils.inputs(struct).size(); @@ -399,33 +396,26 @@ private static Boolean[] getParameterOptionality(Method m, Class opType, if (opMethodHasOptionals && !fMethodsWithOptionals.isEmpty()) { problems.add(new ValidityProblem( "Both the OpMethod and its op type have optional parameters!")); - return generateAllRequiredArray.apply(opParams); + return ReductionUtils.generateAllRequiredArray(opParams); } if (fMethodsWithOptionals.size() > 1) { problems.add(new ValidityProblem( "Multiple methods from the op type have optional parameters!")); - return generateAllRequiredArray.apply(opParams); + return ReductionUtils.generateAllRequiredArray(opParams); } // return the optionality of each parameter of the Op if (opMethodHasOptionals) return getOpMethodOptionals(m, opParams); - if (fMethodsWithOptionals.size() > 0) return findParameterOptionality.apply( + if (fMethodsWithOptionals.size() > 0) return ReductionUtils.findParameterOptionality( fMethodsWithOptionals.get(0)); - return generateAllRequiredArray.apply(opParams); + return ReductionUtils.generateAllRequiredArray(opParams); } - private static Function generateAllRequiredArray = - num -> { - Boolean[] arr = new Boolean[num]; - Arrays.fill(arr, false); - return arr; - }; - private static Boolean[] getOpMethodOptionals(Method m, int opParams) { int[] paramIndex = mapFunctionalParamsToIndices(m.getParameters()); - Boolean[] arr = generateAllRequiredArray.apply(opParams); + Boolean[] arr = ReductionUtils.generateAllRequiredArray(opParams); // check parameters on m - Boolean[] mOptionals = findParameterOptionality.apply(m); + Boolean[] mOptionals = ReductionUtils.findParameterOptionality(m); for(int i = 0; i < arr.length; i++) { int index = paramIndex[i]; if (index == -1) continue; @@ -458,22 +448,4 @@ private static int[] mapFunctionalParamsToIndices(Parameter[] parameters) { return paramNo; } - private static List fMethodsWithOptional(Class opType) { - Method superFMethod = SimplificationUtils.findFMethod(Types.raw(opType)); - List methods = new ArrayList<>(Arrays.asList(opType.getClass().getMethods())); - methods.add(superFMethod); - return methods.parallelStream() // - .filter(m -> m.getName().equals(superFMethod.getName())) // - .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // - .filter(m -> hasOptionalAnntotations.apply(m)) // - .collect(Collectors.toList()); - } - - private static Function findParameterOptionality = - m -> Arrays.stream(m.getParameters()).map(p -> p.isAnnotationPresent( - Optional.class)).toArray(Boolean[]::new); - - private static Function hasOptionalAnntotations = - m -> Arrays.stream(m.getParameters()).anyMatch(p -> p.isAnnotationPresent( - Optional.class)); } \ No newline at end of file diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java index ae4c92e91..0fc08cfa7 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java @@ -6,9 +6,12 @@ import java.lang.reflect.Method; import java.util.Arrays; import java.util.List; +import java.util.function.Function; +import java.util.stream.Collectors; import java.util.stream.Stream; import org.scijava.ops.OpUtils; +import org.scijava.ops.simplify.SimplificationUtils; import org.scijava.ops.simplify.Simplifier; import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; @@ -280,4 +283,29 @@ private static String generateSignature(Method m) { return sb.toString(); } + public static Boolean hasOptionalAnnotations(Method m) { + return Arrays.stream(m.getParameters()).anyMatch(p -> p.isAnnotationPresent( + Optional.class)); + } + + public static Boolean[] findParameterOptionality(Method m) { + return Arrays.stream(m.getParameters()).map(p -> p.isAnnotationPresent( + Optional.class)).toArray(Boolean[]::new); + } + + public static List fMethodsWithOptional(Class opClass) { + Method superFMethod = SimplificationUtils.findFMethod(opClass); + return Arrays.stream(opClass.getMethods()) // + .filter(m -> m.getName().equals(superFMethod.getName())) // + .filter(m -> m.getParameterCount() == superFMethod.getParameterCount()) // + .filter(m -> hasOptionalAnnotations(m)) // + .collect(Collectors.toList()); + } + + public static Boolean[] generateAllRequiredArray(int num) { + Boolean[] arr = new Boolean[num]; + Arrays.fill(arr, false); + return arr; + } + } From 89634d2a50770b090cdc588dafc573904ec69d6e Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Thu, 3 Jun 2021 14:21:10 -0500 Subject: [PATCH 17/24] Add names field to OpInfo Unfortuantely, this results in some API changes in OpEnvironment. --- .../java/org/scijava/ops/OpEnvironment.java | 11 +++---- .../src/main/java/org/scijava/ops/OpInfo.java | 5 +++- .../ops/impl/DefaultOpEnvironment.java | 30 +++++++++---------- .../scijava/ops/matcher/OpAdaptationInfo.java | 5 ++++ .../org/scijava/ops/matcher/OpClassInfo.java | 13 ++++++-- .../org/scijava/ops/matcher/OpFieldInfo.java | 9 +++++- .../org/scijava/ops/matcher/OpMethodInfo.java | 9 +++++- .../org/scijava/ops/reduce/ReducedOpInfo.java | 5 ++++ .../ops/simplify/SimplifiedOpInfo.java | 5 ++++ .../org/scijava/ops/OpEnvironmentTest.java | 10 +++---- 10 files changed, 70 insertions(+), 32 deletions(-) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/OpEnvironment.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/OpEnvironment.java index e52e31c10..8251a0cce 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/OpEnvironment.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/OpEnvironment.java @@ -120,23 +120,24 @@ default OpBuilder op(final String opName) { * Creates an {@link OpInfo} from an {@link Class}. * * @param opClass + * @param names the comma-delimited set of names under which this Op is known * @return an {@link OpInfo} which can make instances of {@code opClass} */ - OpInfo opify(Class opClass); + OpInfo opify(Class opClass, String names); /** * Creates an {@link OpInfo} from an {@link Class} with the given priority. * * @param opClass + * @param names the comma-delimited set of names under which this Op is known * @param priority - the assigned priority of the Op. * @return an {@link OpInfo} which can make instances of {@code opClass} */ - OpInfo opify(Class opClass, double priority); + OpInfo opify(Class opClass, String names, double priority); /** - * Makes the {@link OpInfo} {@code info} known to this {@link OpEnvironment} under the name {@code name} + * Makes the {@link OpInfo} {@code info} known to this {@link OpEnvironment} * @param info - * @param name */ - void register(OpInfo info, String name); + void register(OpInfo info); } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/OpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/OpInfo.java index 562d5f856..cd499b38e 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/OpInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/OpInfo.java @@ -30,7 +30,10 @@ public interface OpInfo extends Comparable { /** Gets the associated {@link Struct} metadata. */ Struct struct(); - + + /** Gets the comma-delimited set of names used to identify the Op */ + String names(); + /** Describes whether this Op can be simplified. */ boolean isSimplifiable(); diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java index b8e5da4b9..c84079c34 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java @@ -199,19 +199,19 @@ public T bakeLambdaType(final T op, Type reifiedType) { } @Override - public OpInfo opify(final Class opClass) { - return opify(opClass, Priority.NORMAL); + public OpInfo opify(final Class opClass, String names) { + return opify(opClass, names, Priority.NORMAL); } @Override - public OpInfo opify(final Class opClass, final double priority) { - return new OpClassInfo(opClass, priority, opClass.getAnnotation(Unsimplifiable.class) == null); + public OpInfo opify(final Class opClass, final String names, final double priority) { + return new OpClassInfo(opClass, names, priority, opClass.getAnnotation(Unsimplifiable.class) == null); } @Override - public void register(final OpInfo info, final String name) { + public void register(final OpInfo info) { if (opDirectory == null) initOpDirectory(); - addToOpIndex(info, name); + addToOpIndex(info); } @SuppressWarnings("unchecked") @@ -684,8 +684,8 @@ private void initOpDirectory() { for (final PluginInfo pluginInfo : pluginService.getPluginsOfType(Op.class)) { try { final Class opClass = pluginInfo.loadClass(); - OpInfo opInfo = new OpClassInfo(opClass); - addToOpIndex(opInfo, pluginInfo.getName()); + OpInfo opInfo = new OpClassInfo(opClass, pluginInfo.getName()); + addToOpIndex(opInfo); boolean hasOptional = OpUtils.inputs(opInfo.struct()).parallelStream() // .anyMatch(m -> opInfo.isOptional(m)); // if (hasOptional) { @@ -706,9 +706,9 @@ private void initOpDirectory() { if (!isStatic && instance == null) { instance = field.getDeclaringClass().newInstance(); } - OpInfo opInfo = new OpFieldInfo(isStatic ? null : instance, field); String names = field.getAnnotation(OpField.class).names(); - addToOpIndex(opInfo, names); + OpInfo opInfo = new OpFieldInfo(isStatic ? null : instance, field, names); + addToOpIndex(opInfo); boolean hasOptional = OpUtils.inputs(opInfo.struct()).parallelStream() // .anyMatch(m -> opInfo.isOptional(m)); // if (hasOptional) { @@ -717,9 +717,9 @@ private void initOpDirectory() { } final List methods = ClassUtils.getAnnotatedMethods(c, OpMethod.class); for (final Method method: methods) { - OpInfo opInfo = new OpMethodInfo(method); String names = method.getAnnotation(OpMethod.class).names(); - addToOpIndex(opInfo, names); + OpInfo opInfo = new OpMethodInfo(method, names); + addToOpIndex(opInfo); boolean hasOptional = OpUtils.inputs(opInfo.struct()).parallelStream() // .anyMatch(m -> opInfo.isOptional(m)); // if (hasOptional) { @@ -748,12 +748,12 @@ private void initOpDirectory() { // add a ReducedOpInfo for all possible reductions // TODO: how to find the names? for (int i = 1; i <= numReductions; i++) { - addToOpIndex(reducer.reduce(info, i), names); + addToOpIndex(reducer.reduce(info, i)); } }; - private void addToOpIndex(final OpInfo opInfo, final String opNames) { - String[] parsedOpNames = OpUtils.parseOpNames(opNames); + private void addToOpIndex(final OpInfo opInfo) { + String[] parsedOpNames = OpUtils.parseOpNames(opInfo.names()); if (parsedOpNames == null || parsedOpNames.length == 0) { log.error("Skipping Op " + opInfo.implementationName() + ":\n" + "Op implementation must provide name."); return; diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpAdaptationInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpAdaptationInfo.java index c9db86d41..efaa87e31 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpAdaptationInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpAdaptationInfo.java @@ -60,6 +60,11 @@ public Struct struct() { return struct; } + @Override + public String names() { + return srcInfo.names(); + } + // we want the original op to have priority over this one. @Override public double priority() { diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java index 80936da9f..aebfb9f53 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpClassInfo.java @@ -64,23 +64,25 @@ public class OpClassInfo implements OpInfo { private final Class opClass; private Struct struct; + private final String names; private ValidityException validityException; private final double priority; private final Boolean[] paramOptionality; private final boolean simplifiable; - public OpClassInfo(final Class opClass) { - this(opClass, priorityFromAnnotation(opClass), simplifiableFromAnnotation( + public OpClassInfo(final Class opClass, final String names) { + this(opClass, names, priorityFromAnnotation(opClass), simplifiableFromAnnotation( opClass)); } - public OpClassInfo(final Class opClass, final double priority, + public OpClassInfo(final Class opClass, final String names, final double priority, final boolean simplifiable) { final List problems = new ArrayList<>(); this.opClass = opClass; + this.names = names; try { struct = ParameterStructs.structOf(opClass); OpUtils.checkHasSingleOutput(struct); @@ -114,6 +116,11 @@ public Struct struct() { return struct; } + @Override + public String names() { + return names; + } + @Override public double priority() { return priority; diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java index 801817f90..bffdf7f03 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpFieldInfo.java @@ -59,6 +59,7 @@ public class OpFieldInfo implements OpInfo { private final Object instance; private final Field field; + private final String names; private Struct struct; private ValidityException validityException; @@ -66,9 +67,10 @@ public class OpFieldInfo implements OpInfo { private final Boolean[] paramOptionality; private final boolean simplifiable; - public OpFieldInfo(final Object instance, final Field field) { + public OpFieldInfo(final Object instance, final Field field, final String names) { this.instance = instance; this.field = field; + this.names = names; if (Modifier.isStatic(field.getModifiers())) { // Field is static; instance must be null. @@ -123,6 +125,11 @@ public Struct struct() { return struct; } + @Override + public String names() { + return names; + } + @Override public double priority() { final OpField opField = field.getAnnotation(OpField.class); diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java index ce0c85b25..9a5c4c443 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java @@ -77,12 +77,13 @@ public class OpMethodInfo implements OpInfo { private final Method method; private Type opType; private Struct struct; + private final String names; private Boolean[] paramOptionality; private final ValidityException validityException; private final boolean simplifiable; - public OpMethodInfo(final Method method) { + public OpMethodInfo(final Method method, final String names) { final List problems = new ArrayList<>(); // Reject all non public methods if (!Modifier.isPublic(method.getModifiers())) { @@ -97,6 +98,7 @@ public OpMethodInfo(final Method method) { " must be static.")); } this.method = method; + this.names = names; // we cannot simplify this op iff it has the Unsimplifiable annotation. simplifiable = method.getAnnotation(Unsimplifiable.class) == null; try { @@ -134,6 +136,11 @@ public Struct struct() { return struct; } + @Override + public String names() { + return names; + } + @Override public double priority() { final OpMethod opMethod = method.getAnnotation(OpMethod.class); diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java index dde338213..cc0d0ee9a 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java @@ -47,6 +47,11 @@ public Struct struct() { return struct; } + @Override + public String names() { + return srcInfo.names(); + } + @Override public boolean isSimplifiable() { return true; diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplifiedOpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplifiedOpInfo.java index 8219be323..7cd8c14ab 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplifiedOpInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/simplify/SimplifiedOpInfo.java @@ -64,6 +64,11 @@ public Struct struct() { return struct; } + @Override + public String names() { + return srcInfo.names(); + } + @Override public double priority() { return priority; diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/OpEnvironmentTest.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/OpEnvironmentTest.java index 919d6e5cf..0d05a45a3 100644 --- a/scijava/scijava-ops/src/test/java/org/scijava/ops/OpEnvironmentTest.java +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/OpEnvironmentTest.java @@ -7,9 +7,7 @@ import org.junit.Test; import org.scijava.Priority; import org.scijava.ops.function.Producer; -import org.scijava.ops.matcher.OpClassInfo; import org.scijava.param.Parameter; -import org.scijava.struct.ItemIO; import org.scijava.types.GenericTyped; import org.scijava.types.Nil; @@ -42,7 +40,7 @@ public void testBakeType() { @Test public void testClassOpification() { - OpInfo opifyOpInfo = ops.env().opify(OpifyOp.class); + OpInfo opifyOpInfo = ops.env().opify(OpifyOp.class, "foo"); Assert.assertEquals(OpifyOp.class.getName(), opifyOpInfo.implementationName()); // assert default priority Assert.assertEquals(Priority.NORMAL, opifyOpInfo.priority(), 0.); @@ -50,7 +48,7 @@ public void testClassOpification() { @Test public void testClassOpificationWithPriority() { - OpInfo opifyOpInfo = ops.env().opify(OpifyOp.class, Priority.HIGH); + OpInfo opifyOpInfo = ops.env().opify(OpifyOp.class, "foo", Priority.HIGH); Assert.assertEquals(OpifyOp.class.getName(), opifyOpInfo.implementationName()); // assert default priority Assert.assertEquals(Priority.HIGH, opifyOpInfo.priority(), 0.); @@ -59,8 +57,8 @@ public void testClassOpificationWithPriority() { @Test public void testRegister() { String opName = "test.opifyOp"; - OpInfo opifyOpInfo = ops.env().opify(OpifyOp.class, Priority.HIGH); - ops.env().register(opifyOpInfo, opName); + OpInfo opifyOpInfo = ops.env().opify(OpifyOp.class, opName, Priority.HIGH); + ops.env().register(opifyOpInfo); String actual = ops.op(opName).input().outType(String.class).create(); From 602bf2fcb556fe6c6622f021321c05b1cf62d754 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Thu, 3 Jun 2021 15:00:39 -0500 Subject: [PATCH 18/24] Move fail-fast reduction logic to reduceInfo Reduces duplicated code --- .../ops/impl/DefaultOpEnvironment.java | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java index c84079c34..4d1e00c34 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java @@ -686,11 +686,7 @@ private void initOpDirectory() { final Class opClass = pluginInfo.loadClass(); OpInfo opInfo = new OpClassInfo(opClass, pluginInfo.getName()); addToOpIndex(opInfo); - boolean hasOptional = OpUtils.inputs(opInfo.struct()).parallelStream() // - .anyMatch(m -> opInfo.isOptional(m)); // - if (hasOptional) { - reduceInfo.accept(opInfo, pluginInfo.getName()); - } + reduceInfo.accept(opInfo, pluginInfo.getName()); } catch (InstantiableException exc) { log.error("Can't load class from plugin info: " + pluginInfo.toString(), exc); } @@ -709,22 +705,14 @@ private void initOpDirectory() { String names = field.getAnnotation(OpField.class).names(); OpInfo opInfo = new OpFieldInfo(isStatic ? null : instance, field, names); addToOpIndex(opInfo); - boolean hasOptional = OpUtils.inputs(opInfo.struct()).parallelStream() // - .anyMatch(m -> opInfo.isOptional(m)); // - if (hasOptional) { - reduceInfo.accept(opInfo, names); - } + reduceInfo.accept(opInfo, names); } final List methods = ClassUtils.getAnnotatedMethods(c, OpMethod.class); for (final Method method: methods) { String names = method.getAnnotation(OpMethod.class).names(); OpInfo opInfo = new OpMethodInfo(method, names); addToOpIndex(opInfo); - boolean hasOptional = OpUtils.inputs(opInfo.struct()).parallelStream() // - .anyMatch(m -> opInfo.isOptional(m)); // - if (hasOptional) { - reduceInfo.accept(opInfo, names); - } + reduceInfo.accept(opInfo, names); } } catch (InstantiableException | InstantiationException | IllegalAccessException exc) { log.error("Can't load class from plugin info: " + pluginInfo.toString(), exc); @@ -733,6 +721,11 @@ private void initOpDirectory() { } private final BiConsumer reduceInfo = (info, names) -> { + // if there is nothing to reduce, end quickly + boolean hasOptional = OpUtils.inputs(info.struct()).parallelStream() // + .anyMatch(m -> info.isOptional(m)); // + if (!hasOptional) return; + // find a InfoReducer capable of reducing info Optional suitableReducer = infoReducers .parallelStream().filter(reducer -> reducer.canReduce(info)).findAny(); From 8c4210253ca429c050352294e359204664b41584 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Thu, 3 Jun 2021 15:39:36 -0500 Subject: [PATCH 19/24] Separate Op parsing into Consumers This makes it easier to parallelize later --- .../ops/impl/DefaultOpEnvironment.java | 93 +++++++++++-------- 1 file changed, 53 insertions(+), 40 deletions(-) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java index 4d1e00c34..a518f56f2 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/impl/DefaultOpEnvironment.java @@ -40,7 +40,6 @@ import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -75,13 +74,13 @@ import org.scijava.ops.matcher.OpAdaptationInfo; import org.scijava.ops.matcher.OpCandidate; import org.scijava.ops.matcher.OpCandidate.StatusCode; -import org.scijava.ops.reduce.InfoReducer; import org.scijava.ops.matcher.OpClassInfo; import org.scijava.ops.matcher.OpFieldInfo; import org.scijava.ops.matcher.OpMatcher; import org.scijava.ops.matcher.OpMatchingException; import org.scijava.ops.matcher.OpMethodInfo; import org.scijava.ops.matcher.OpRef; +import org.scijava.ops.reduce.InfoReducer; import org.scijava.ops.simplify.InfoSimplificationGenerator; import org.scijava.ops.simplify.SimplificationUtils; import org.scijava.ops.simplify.SimplifiedOpInfo; @@ -93,7 +92,6 @@ import org.scijava.plugin.Parameter; import org.scijava.plugin.PluginInfo; import org.scijava.plugin.PluginService; -import org.scijava.plugin.SciJavaPlugin; import org.scijava.struct.ItemIO; import org.scijava.types.Nil; import org.scijava.types.TypeService; @@ -676,51 +674,28 @@ private OpRef inferOpRef(Type type, String name, Map, Type> type return new OpRef(name, type, mappedOutputs[0], mappedInputs); } + /** + * Initializes the Op Directory. There are two phases: + *
      + *
    1. Ops written as Classes are discovered + *
    2. Ops written as Fields and Methods are discovered via {@link OpCollection} annotations + *
    + * + * TODO: can this be done in parallel? + */ private void initOpDirectory() { opDirectory = new HashMap<>(); initInfoReducers(); // Add regular Ops - for (final PluginInfo pluginInfo : pluginService.getPluginsOfType(Op.class)) { - try { - final Class opClass = pluginInfo.loadClass(); - OpInfo opInfo = new OpClassInfo(opClass, pluginInfo.getName()); - addToOpIndex(opInfo); - reduceInfo.accept(opInfo, pluginInfo.getName()); - } catch (InstantiableException exc) { - log.error("Can't load class from plugin info: " + pluginInfo.toString(), exc); - } - } + pluginService.getPluginsOfType(Op.class) // + .stream().forEach(parseOpClass); // Add Ops contained in an OpCollection - for (final PluginInfo pluginInfo : pluginService.getPluginsOfType(OpCollection.class)) { - try { - final Class c = pluginInfo.loadClass(); - final List fields = ClassUtils.getAnnotatedFields(c, OpField.class); - Object instance = null; - for (Field field : fields) { - final boolean isStatic = Modifier.isStatic(field.getModifiers()); - if (!isStatic && instance == null) { - instance = field.getDeclaringClass().newInstance(); - } - String names = field.getAnnotation(OpField.class).names(); - OpInfo opInfo = new OpFieldInfo(isStatic ? null : instance, field, names); - addToOpIndex(opInfo); - reduceInfo.accept(opInfo, names); - } - final List methods = ClassUtils.getAnnotatedMethods(c, OpMethod.class); - for (final Method method: methods) { - String names = method.getAnnotation(OpMethod.class).names(); - OpInfo opInfo = new OpMethodInfo(method, names); - addToOpIndex(opInfo); - reduceInfo.accept(opInfo, names); - } - } catch (InstantiableException | InstantiationException | IllegalAccessException exc) { - log.error("Can't load class from plugin info: " + pluginInfo.toString(), exc); - } - } + pluginService.getPluginsOfType(OpCollection.class) // + .stream().forEach(parseOpCollection); } - private final BiConsumer reduceInfo = (info, names) -> { + private final Consumer reduceInfo = (info) -> { // if there is nothing to reduce, end quickly boolean hasOptional = OpUtils.inputs(info.struct()).parallelStream() // .anyMatch(m -> info.isOptional(m)); // @@ -745,6 +720,44 @@ private void initOpDirectory() { } }; + private final Consumer> parseOpClass = (pluginInfo) -> { + try { + final Class opClass = pluginInfo.loadClass(); + OpInfo opInfo = new OpClassInfo(opClass, pluginInfo.getName()); + addToOpIndex(opInfo); + reduceInfo.accept(opInfo); + } catch (InstantiableException exc) { + log.error("Can't load class from plugin info: " + pluginInfo.toString(), exc); + } + }; + + private final Consumer> parseOpCollection = pluginInfo -> { + try { + final Class c = pluginInfo.loadClass(); + final List fields = ClassUtils.getAnnotatedFields(c, OpField.class); + Object instance = null; + for (Field field : fields) { + final boolean isStatic = Modifier.isStatic(field.getModifiers()); + if (!isStatic && instance == null) { + instance = field.getDeclaringClass().newInstance(); + } + String names = field.getAnnotation(OpField.class).names(); + OpInfo opInfo = new OpFieldInfo(isStatic ? null : instance, field, names); + addToOpIndex(opInfo); + reduceInfo.accept(opInfo); + } + final List methods = ClassUtils.getAnnotatedMethods(c, OpMethod.class); + for (final Method method: methods) { + String names = method.getAnnotation(OpMethod.class).names(); + OpInfo opInfo = new OpMethodInfo(method, names); + addToOpIndex(opInfo); + reduceInfo.accept(opInfo); + } + } catch (InstantiableException | InstantiationException | IllegalAccessException exc) { + log.error("Can't load class from plugin info: " + pluginInfo.toString(), exc); + } + }; + private void addToOpIndex(final OpInfo opInfo) { String[] parsedOpNames = OpUtils.parseOpNames(opInfo.names()); if (parsedOpNames == null || parsedOpNames.length == 0) { From 809d0e70925193d2f2dac7456cd4a1f1f514a03a Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Fri, 4 Jun 2021 10:59:11 -0500 Subject: [PATCH 20/24] Denote Producer as a FunctionalInterface Not only IS it a functional interface, but our findFunctionalInterface logic will delegate to Supplier otherwise (which is bad) --- .../src/main/java/org/scijava/ops/function/Producer.java | 1 + 1 file changed, 1 insertion(+) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/function/Producer.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/function/Producer.java index d91f9b28d..2edfe98c3 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/function/Producer.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/function/Producer.java @@ -18,6 +18,7 @@ * @param * The type of objects produced. */ +@FunctionalInterface public interface Producer extends Supplier { O create(); From 9d81f0581e52727ac8d8d10b95decd04281e4ef3 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Fri, 4 Jun 2021 11:13:20 -0500 Subject: [PATCH 21/24] Improve functional method determination We need the names to correctly implement the interfaces/call the original method --- .../src/main/java/org/scijava/ops/reduce/ReductionUtils.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java index 0fc08cfa7..3da40d7a7 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java @@ -213,7 +213,10 @@ private static String createFunctionalMethod(ReducedOpInfo info) { StringBuilder sb = new StringBuilder(); // determine the name of the functional method - Method m = ParameterStructs.singularAbstractMethod(Types.raw(info.opType())); + Class fIface = ParameterStructs.findFunctionalInterface(Types.raw(info.opType())); + Method m = ParameterStructs.singularAbstractMethod(fIface); + Class srcFIface = ParameterStructs.findFunctionalInterface(Types.raw(info.srcInfo().opType())); + Method srcM = ParameterStructs.singularAbstractMethod(srcFIface); // determine the name of the output: String opOutput = "out"; From 97f84826c340ec7bcbf2781feadcbc9f7f5db8c5 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Fri, 4 Jun 2021 11:20:49 -0500 Subject: [PATCH 22/24] Prevent multiple class declarations --- .../org/scijava/ops/matcher/OpMethodInfo.java | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java index 9a5c4c443..23f8275b4 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java @@ -205,8 +205,29 @@ private Object javassistOp(Method m, List dependencies) // Create wrapper class String className = formClassName(m); - CtClass cc = pool.makeClass(className); + Class c; + try { + // use javassist to create the class + CtClass cc = constructOpMethodWrapper(pool, className, m); + c = cc.toClass(MethodHandles.lookup()); + } + catch (RuntimeException e) { + // the OpMethod has already been created - find it + c= Class.forName(className); + } + + // Return Op instance + List> depMembers = OpUtils.dependencies(struct()); + Class[] depClasses = depMembers.stream().map(dep -> dep.getRawType()) + .toArray(Class[]::new); + return c.getDeclaredConstructor(depClasses).newInstance(dependencies + .toArray()); + } + private CtClass constructOpMethodWrapper(ClassPool pool, String className, + Method m) throws Throwable + { + CtClass cc = pool.makeClass(className); // Add implemented interface CtClass jasOpType = pool.get(Types.raw(opType).getName()); cc.addInterface(jasOpType); @@ -226,13 +247,7 @@ private Object javassistOp(Method m, List dependencies) // add functional interface method CtMethod functionalMethod = CtNewMethod.make(createFunctionalMethod(m), cc); cc.addMethod(functionalMethod); - - // Return Op instance - Class[] depClasses = depMembers.stream().map(dep -> dep.getRawType()) - .toArray(Class[]::new); - Class c = cc.toClass(MethodHandles.lookup()); - return c.getDeclaredConstructor(depClasses).newInstance(dependencies - .toArray()); + return cc; } private String formClassName(Method m) { From 8c3801d61c5584f4d7f1923499a75054d6b27be6 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Thu, 3 Jun 2021 17:18:13 -0500 Subject: [PATCH 23/24] test reduction in methods with dependencies These tests show that reduction with dependencies works regardless of where the Dependencies are in the signature, which is uber cool! --- .../org/scijava/ops/matcher/OpMethodInfo.java | 4 +- .../org/scijava/ops/reduce/ReducedOpInfo.java | 13 +++- .../scijava/ops/reduce/ReductionUtils.java | 2 +- .../reduce/ReductionWithDependenciesTest.java | 65 +++++++++++++++++++ 4 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/ReductionWithDependenciesTest.java diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java index 23f8275b4..30a327185 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/matcher/OpMethodInfo.java @@ -438,10 +438,10 @@ private static Boolean[] getOpMethodOptionals(Method m, int opParams) { Boolean[] arr = ReductionUtils.generateAllRequiredArray(opParams); // check parameters on m Boolean[] mOptionals = ReductionUtils.findParameterOptionality(m); - for(int i = 0; i < arr.length; i++) { + for(int i = 0; i < mOptionals.length; i++) { int index = paramIndex[i]; if (index == -1) continue; - arr[i] |= mOptionals[index]; + arr[index] |= mOptionals[i]; } return arr; } diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java index cc0d0ee9a..38deeae9e 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java @@ -1,12 +1,13 @@ package org.scijava.ops.reduce; import java.lang.reflect.AnnotatedElement; -import java.lang.reflect.Modifier; import java.lang.reflect.Type; import java.util.List; import java.util.Objects; +import org.scijava.ops.OpDependencyMember; import org.scijava.ops.OpInfo; +import org.scijava.ops.OpUtils; import org.scijava.param.Optional; import org.scijava.param.ParameterStructs; import org.scijava.param.ValidityException; @@ -68,6 +69,16 @@ public String implementationName() { return srcInfo.implementationName() + "Reduction" + paramsReduced; } + /** + * Gets the op's dependencies on other ops. NB the reduction wrapper has no + * dependencies, but the Op itself might. So the dependencies are actually + * reflected in {@code srcInfo} + */ + @Override + public List> dependencies() { + return OpUtils.dependencies(srcInfo().struct()); + } + @Override public StructInstance createOpInstance(List dependencies) { final Object op = srcInfo.createOpInstance(dependencies).object(); diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java index 3da40d7a7..c577bc097 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReductionUtils.java @@ -230,7 +230,7 @@ private static String createFunctionalMethod(ReducedOpInfo info) { if (OpUtils.hasPureOutput(info)) { sb.append(OpUtils.outputType(info.struct()).getTypeName() + " " + opOutput + " = "); } - sb.append("op." + m.getName() + "("); + sb.append("op." + srcM.getName() + "("); int i; List> totalArguments = OpUtils.inputs(info.srcInfo().struct()); int totalArgs = OpUtils.inputs(info.srcInfo().struct()).size(); diff --git a/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/ReductionWithDependenciesTest.java b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/ReductionWithDependenciesTest.java new file mode 100644 index 000000000..2cb99ac4b --- /dev/null +++ b/scijava/scijava-ops/src/test/java/org/scijava/ops/reduce/ReductionWithDependenciesTest.java @@ -0,0 +1,65 @@ +package org.scijava.ops.reduce; + +import java.util.function.Function; + +import org.junit.Test; +import org.junit.Assert; +import org.scijava.ops.AbstractTestEnvironment; +import org.scijava.ops.OpDependency; +import org.scijava.ops.OpMethod; +import org.scijava.ops.core.OpCollection; +import org.scijava.ops.function.Producer; +import org.scijava.param.Optional; +import org.scijava.plugin.Plugin; + +@Plugin(type = OpCollection.class) +public class ReductionWithDependenciesTest extends AbstractTestEnvironment{ + + @OpMethod(names = "test.fooDependency", type = Producer.class) + public static Double bar() { + return 5.; + } + + @OpMethod(names = "test.optionalWithDependency", type = Function.class) + public static Double foo(@OpDependency(name = "test.fooDependency") Producer bar, @Optional Double opt) { + if (opt == null) opt = 0.; + return bar.create() + opt; + } + + @OpMethod(names = "test.optionalWithDependency2", type = Function.class) + public static Double foo(@Optional Double opt, @OpDependency(name = "test.fooDependency") Producer bar) { + if (opt == null) opt = 0.; + return bar.create() + opt; + } + + @Test + public void testDependencyFirstMethodWithOptional() { + Double opt = 7.; + Double o = ops.op("test.optionalWithDependency").input(opt).outType(Double.class).apply(); + Double expected = 12.; + Assert.assertEquals(expected, o); + } + + @Test + public void testDependencyFirstMethodWithoutOptional() { + Double o = ops.op("test.optionalWithDependency").input().outType(Double.class).create(); + Double expected = 5.; + Assert.assertEquals(expected, o); + } + + @Test + public void testDependencySecondMethodWithOptional() { + Double opt = 7.; + Double o = ops.op("test.optionalWithDependency2").input(opt).outType(Double.class).apply(); + Double expected = 12.; + Assert.assertEquals(expected, o); + } + + @Test + public void testDependencySecondMethodWithoutOptional() { + Double o = ops.op("test.optionalWithDependency2").input().outType(Double.class).create(); + Double expected = 5.; + Assert.assertEquals(expected, o); + } + +} From 7d1d7d4d7e9408a6f56e4886a34888411f67b889 Mon Sep 17 00:00:00 2001 From: Gabriel Selzer Date: Fri, 4 Jun 2021 13:19:47 -0500 Subject: [PATCH 24/24] Remove comment about non-public reduction This check was never necessary --- .../src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java index 38deeae9e..94abf5049 100644 --- a/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java +++ b/scijava/scijava-ops/src/main/java/org/scijava/ops/reduce/ReducedOpInfo.java @@ -82,10 +82,6 @@ public List> dependencies() { @Override public StructInstance createOpInstance(List dependencies) { final Object op = srcInfo.createOpInstance(dependencies).object(); -// if (!Modifier.isPublic(srcInfo.opType().getClass().getModifiers())) { -// throw new IllegalArgumentException("Op " + srcInfo.opType() + -// " is not public; only Ops backed by public classes can be reduced!"); -// } try { Object reducedOp = ReductionUtils.javassistOp(op, this); return struct().createInstance(reducedOp);