-
Notifications
You must be signed in to change notification settings - Fork 214
Add Session Result class #167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7202ea3
78384d7
e4cf856
786e3fc
4500b56
666f434
91b63ac
6754d4a
c96ec0a
64b6f2e
1193412
e95ae47
701f7f0
0a771b3
598da62
f67d6e2
22f0a05
58ff935
3dc6def
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -126,6 +126,66 @@ public GraphOperation operation(String name) { | |
} | ||
} | ||
|
||
|
||
/** | ||
* Returns the operation (node in the Graph) with the provided name. | ||
* <p> | ||
* Or throws an {@code IllegalArgumentException} if no such operation exists in the Graph. | ||
* | ||
* @param name name of the operation to look for | ||
* @return operation in the graph with this name | ||
* @see #operation(String) | ||
*/ | ||
public GraphOperation operationOrError(String name) { | ||
GraphOperation op = operation(name); | ||
if (op == null) { | ||
throw new IllegalArgumentException("No Operation named [" + name + "] in the Graph"); | ||
} | ||
return op; | ||
} | ||
|
||
/** | ||
* Returns the {@code index}-th output of {@code operation}. | ||
* Throws {@code IllegalArgumentException} if the operation is not found, or does not have an output at {@code index}. | ||
* | ||
* @param operation The operation to get the output of. | ||
* @param index The index of the output to get. | ||
* @return The {@code index}-th output of {@code operation}. | ||
*/ | ||
public Output<?> getOutput(String operation, int index){ | ||
GraphOperation graphOp = operationOrError(operation); | ||
if(index < 0 || index >= graphOp.numOutputs()){ | ||
throw new IllegalArgumentException("Index out of bounds for operation " + operation + | ||
". Operation has " + graphOp.numOutputs() + " outputs"); | ||
} | ||
|
||
return graphOp.output(index); | ||
} | ||
|
||
/** | ||
* Returns the output specified by {@code output}. | ||
* Will try to parse the output index from {@code output}. | ||
* I.e. {@code "scope/op:2"} will get the 2nd (0-indexed) output of {@code scope/op}. | ||
* Otherwise, will return the 0th output. | ||
* | ||
* @param output The operation to get the output of, with the index optionally specified by colon. | ||
* @return The output specified by {@code output}. | ||
*/ | ||
@SuppressWarnings("rawtypes") | ||
public Output<?> getOutput(String output) { | ||
int colon = output.lastIndexOf(':'); | ||
if (colon == -1 || colon == output.length() - 1) { | ||
return new Output(operationOrError(output), 0); | ||
} | ||
try { | ||
String op = output.substring(0, colon); | ||
int index = Integer.parseInt(output.substring(colon + 1)); | ||
return new Output(operationOrError(op), index); | ||
} catch (NumberFormatException e) { | ||
return new Output(operationOrError(output), 0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fallback case seems odd, as it doesn't log anything when it's likely to be programmer error if it's triggered right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved that from Session, I don't think it's too odd? It's less about the actual number and more about if you have an op name like Logging it would be a good idea, but it doesn't look like there's any logging currently set up. Is there one I can use? |
||
} | ||
} | ||
|
||
/** | ||
* Iterator over all the {@link Operation}s in the graph. | ||
* | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why wouldn't this return
null
orOptional<GraphOperation>
rather than throwing? Is it a Kotlin thing?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kind of, it definitely works better from it, eventually I'd want to mark it
@NonNull
. I moved the method here from session, where it did throw like that rather than returning null. Since we already had theoperation
in Graph (which returns null if not found) I added this asoperationOrError
rather than moving the check to any uses.