Skip to content

Restrict Ops to single output #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ctrueden merged 1 commit into from
Jun 19, 2019
Merged

Restrict Ops to single output #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions src/main/java/org/scijava/ops/OpService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,9 @@ private void resolveOpDependencies(Object obj, OpCandidate parentOp) throws OpMa

@SuppressWarnings("unchecked")
public <T> T findOpInstance(final String opName, final Nil<T> specialType, final Nil<?>[] inTypes,
final Nil<?>[] outTypes, final Object... secondaryArgs) {
final OpRef ref = OpRef.fromTypes(opName, toTypes(specialType), toTypes(outTypes), toTypes(inTypes));
final Nil<?> outType, final Object... secondaryArgs) {
final OpRef ref = OpRef.fromTypes(opName, toTypes(specialType), outType != null ? outType.getType() : null, toTypes(
inTypes));
return (T) findOpInstance(opName, ref, secondaryArgs);
}

Expand Down Expand Up @@ -278,14 +279,9 @@ else if (transformation != null)
return op;
}

public <T> T findOp(final String opName, final Nil<T> specialType, final Nil<?>[] inTypes, final Nil<?>[] outTypes,
final Object... secondaryArgs) {
return findOpInstance(opName, specialType, inTypes, outTypes, secondaryArgs);
}

public <T> T findOp(final String opName, final Nil<T> specialType, final Nil<?>[] inTypes, final Nil<?> outType,
final Object... secondaryArgs) {
return findOpInstance(opName, specialType, inTypes, new Nil[] { outType }, secondaryArgs);
return findOpInstance(opName, specialType, inTypes, outType, secondaryArgs);
}

private Type[] toTypes(Nil<?>... nils) {
Expand All @@ -295,11 +291,10 @@ private Type[] toTypes(Nil<?>... nils) {
public Object run(final String opName, final Object... args) {

Nil<?>[] inTypes = Arrays.stream(args).map(arg -> Nil.of(typeService.reify(arg))).toArray(Nil[]::new);
Nil<?>[] outTypes = new Nil<?>[] { new Nil<Object>() {
} };
Nil<?> outType = new Nil<Object>() {};

OpRunner<Object> op = findOpInstance(opName, new Nil<OpRunner<Object>>() {
}, inTypes, outTypes);
}, inTypes, outType);

// TODO change
return op.run(args);
Expand Down Expand Up @@ -329,7 +324,9 @@ public Object run(final String opName, final Object... args) {
* @param name
* @return null if the specified type has no functional method
*/
private OpRef inferOpRef(Type type, String name, Map<TypeVariable<?>, Type> typeVarAssigns) {
private OpRef inferOpRef(Type type, String name, Map<TypeVariable<?>, Type> typeVarAssigns)
throws OpMatchingException
{
List<FunctionalMethodType> fmts = ParameterStructs.getFunctionalMethodTypes(type);
if (fmts == null)
return null;
Expand All @@ -346,7 +343,16 @@ private OpRef inferOpRef(Type type, String name, Map<TypeVariable<?>, Type> type
Type[] mappedInputs = Types.mapVarToTypes(inputs, typeVarAssigns);
Type[] mappedOutputs = Types.mapVarToTypes(outputs, typeVarAssigns);

return new OpRef(name, new Type[] { type }, mappedOutputs, mappedInputs);
final int numOutputs = mappedOutputs.length;
if (numOutputs != 1) {
String error = "Op '" + name + "' of type " + type + " specifies ";
error += numOutputs == 0 //
? "no outputs" //
: "multiple outputs: " + Arrays.toString(outputs);
error += ". This is not supported.";
throw new OpMatchingException(error);
}
return new OpRef(name, new Type[] { type }, mappedOutputs[0], mappedInputs);
}

/**
Expand Down
47 changes: 30 additions & 17 deletions src/main/java/org/scijava/ops/OpUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@

import java.lang.reflect.Type;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.stream.Collectors;

import org.scijava.ops.matcher.DefaultOpTypeMatchingService;
Expand All @@ -41,6 +43,8 @@
import org.scijava.ops.matcher.OpInfo;
import org.scijava.ops.matcher.OpRef;
import org.scijava.param.ParameterMember;
import org.scijava.param.ValidityException;
import org.scijava.param.ValidityProblem;
import org.scijava.struct.Member;
import org.scijava.struct.MemberInstance;
import org.scijava.struct.Struct;
Expand Down Expand Up @@ -139,12 +143,12 @@ public static Type[] inputTypes(OpCandidate candidate) {
return getTypes(inputs(candidate.struct()));
}

public static List<Member<?>> outputs(OpCandidate candidate) {
return outputs(candidate.struct());
public static Member<?> output(OpCandidate candidate) {
return candidate.opInfo().output();
}

public static Type[] outputTypes(OpCandidate candidate) {
return getTypes(outputs(candidate.struct()));
public static Type outputType(OpCandidate candidate) {
return output(candidate).getType();
}

public static List<Member<?>> outputs(final Struct struct) {
Expand All @@ -158,7 +162,17 @@ public static List<MemberInstance<?>> outputs(StructInstance<?> op) {
.filter(memberInstance -> memberInstance.member().isOutput()) //
.collect(Collectors.toList());
}


public static void checkHasSingleOutput(Struct struct) throws ValidityException {
final int numOutputs = OpUtils.outputs(struct).size();
if (numOutputs != 1) {
final String error = numOutputs == 0 //
? "No output parameters specified. Must specify exactly one." //
: "Multiple output parameters specified. Only a single output is allowed.";
throw new ValidityException(Collections.singletonList(new ValidityProblem(error)));
}
}

public static Type[] types(OpCandidate candidate) {
return getTypes(candidate.struct().members());
}
Expand Down Expand Up @@ -238,19 +252,19 @@ public static List<Member<?>> injectableMembers(Struct struct) {

/**
* Checks if incomplete type matching could have occurred. If we have
* several matches that do not have equal output types, output types may not
* several matches that do not have equal output types, the output type may not
* completely match the request as only raw type assignability will be checked
* at the moment.
* @see DefaultOpTypeMatchingService#typesMatch(OpCandidate)
* @param matches
* @return
*/
private static boolean typeCheckingIncomplete(List<OpCandidate> matches) {
Type[] outputTypes = null;
Type outputType = null;
for (OpCandidate match : matches) {
Type[] ts = getTypes(outputs(match));
if (outputTypes == null || Arrays.deepEquals(outputTypes, ts)) {
outputTypes = ts;
Type ts = output(match).getType();
if (outputType == null || Objects.equals(outputType, ts)) {
outputType = ts;
continue;
} else {
return true;
Expand Down Expand Up @@ -404,13 +418,12 @@ public static String opString(final OpInfo info) {
sb.append("\n");
}
sb.append("\t Outputs:\n");
for (final Member<?> arg : info.outputs()) {
sb.append("\t\t");
sb.append(arg.getType().getTypeName());
sb.append(" ");
sb.append(arg.getKey());
sb.append("\n");
}
final Member<?> arg = info.output();
sb.append("\t\t");
sb.append(arg.getType().getTypeName());
sb.append(" ");
sb.append(arg.getKey());
sb.append("\n");
sb.append(")\n");
return sb.toString();
}
Expand Down
42 changes: 13 additions & 29 deletions src/main/java/org/scijava/ops/matcher/DefaultOpTypeMatchingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Predicate;

import org.scijava.Context;
Expand Down Expand Up @@ -239,15 +240,9 @@ private boolean typesPerfectMatch(final OpCandidate candidate) {
i++;
}

Type[] outputTypes = candidate.getRef().getOutTypes();
i = 0;
for (final Type t : OpUtils.outputTypes(candidate)) {
if (outputTypes[i] != null) {
if (!t.equals(outputTypes[i]))
return false;
}
i++;
}
final Type outputType = candidate.getRef().getOutType();
if (Objects.equals(outputType, OpUtils.outputType(candidate)))
return false;

candidate.setStatus(StatusCode.MATCH);
return true;
Expand Down Expand Up @@ -309,37 +304,26 @@ private boolean inputsMatch(final OpCandidate candidate, HashMap<TypeVariable<?>
}

/**
* Checks whether the output types of the candidate match the output types
* of the {@link OpRef}. Sets candidate status code if there are too many,
* to few, or not matching types.
* Checks whether the output type of the candidate matches the output type
* of the {@link OpRef}. Sets candidate status code if they are not matching.
*
* @param candidate
* the candidate to check outputs for
* the candidate to check output for
* @param typeBounds
* possibly predetermined type bounds for type variables
* @return whether the output types match
*/
private boolean outputsMatch(final OpCandidate candidate, HashMap<TypeVariable<?>, TypeVarInfo> typeBounds) {
final Type[] refOutTypes = candidate.getRef().getOutTypes();
if (refOutTypes == null)
final Type refOutType = candidate.getRef().getOutType();
if (refOutType == null)
return true; // no constraints on output types

Type[] candidateOutTypes = OpUtils.outputTypes(candidate);
if (candidateOutTypes.length < refOutTypes.length) {
candidate.setStatus(StatusCode.TOO_FEW_OUTPUTS);
return false;
} else if (candidateOutTypes.length > refOutTypes.length) {
candidate.setStatus(StatusCode.TOO_MANY_OUTPUTS);
return false;
}

int conflictingIndex = MatchingUtils.checkGenericOutputsAssignability(candidateOutTypes, refOutTypes,
typeBounds);
final Type candidateOutType = OpUtils.outputType(candidate);
final int conflictingIndex = MatchingUtils.checkGenericOutputsAssignability(new Type[] { candidateOutType },
new Type[] { refOutType }, typeBounds);
if (conflictingIndex != -1) {
final Type to = refOutTypes[conflictingIndex];
final Type from = candidateOutTypes[conflictingIndex];
candidate.setStatus(StatusCode.OUTPUT_TYPES_DO_NOT_MATCH, //
"request=" + to.getTypeName() + ", actual=" + from.getTypeName());
"request=" + refOutType.getTypeName() + ", actual=" + candidateOutType.getTypeName());
return false;
}
return true;
Expand Down
5 changes: 0 additions & 5 deletions src/main/java/org/scijava/ops/matcher/OpCandidate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ public class OpCandidate {
public static enum StatusCode {
MATCH, //
INVALID_STRUCT, //
TOO_FEW_OUTPUTS, //
TOO_MANY_OUTPUTS,
OUTPUT_TYPES_DO_NOT_MATCH, //
TOO_MANY_ARGS, //
TOO_FEW_ARGS, //
Expand Down Expand Up @@ -168,9 +166,6 @@ public String getStatus() {
sb.append(vp.getMessage());
}
break;
case TOO_FEW_OUTPUTS:
sb.append("Too few outputs");
break;
case OUTPUT_TYPES_DO_NOT_MATCH:
sb.append("Output types do not match");
break;
Expand Down
13 changes: 1 addition & 12 deletions src/main/java/org/scijava/ops/matcher/OpClassInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,13 @@
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Type;
import java.util.List;

import org.scijava.core.Priority;
import org.scijava.ops.OpUtils;
import org.scijava.ops.core.Op;
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.util.Types;
Expand All @@ -61,6 +59,7 @@ public OpClassInfo(final Class<? extends Op> opClass) {
this.opClass = opClass;
try {
struct = ParameterStructs.structOf(opClass);
OpUtils.checkHasSingleOutput(struct);
} catch (ValidityException e) {
validityException = e;
}
Expand All @@ -80,16 +79,6 @@ public Struct struct() {
return struct;
}

@Override
public List<Member<?>> inputs() {
return OpUtils.inputs(struct());
}

@Override
public List<Member<?>> outputs() {
return OpUtils.outputs(struct());
}

@Override
public double priority() {
final Plugin opAnnotation = opClass.getAnnotation(Plugin.class);
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/scijava/ops/matcher/OpFieldInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public OpFieldInfo(final Field field) {
this.field = field;
try {
struct = ParameterStructs.structOf(field.getDeclaringClass(), field);
OpUtils.checkHasSingleOutput(struct);
if (!Modifier.isStatic(field.getModifiers())) {
instance = field.getDeclaringClass().newInstance();
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/scijava/ops/matcher/OpInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ default List<Member<?>> inputs() {
}

/** Gets the op's output parameters. */
default List<Member<?>> outputs() {
return OpUtils.outputs(struct());
default Member<?> output() {
return OpUtils.outputs(struct()).get(0);
}

/** The op's priority. */
Expand Down
Loading