Expose input and output types from Signature#182
Expose input and output types from Signature#182karllessard merged 3 commits intotensorflow:masterfrom
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
|
Thanks @jbentleyEG , looks good but do you have an example on how you actually use it? (maybe adding a small unit test could be great) Also I'm wondering what is better: return types mapped to their tensor name, like you did, or simply return the whole |
|
Agreed, I will re-write it to expose the shape as well. That isn't required for my use case, but I imagine that others will need that. How about just making it a pass-through to the signatureDef method: |
|
@googlebot I signed it! |
That is what I had initially in mind as well, but now I'm thinking that we might want to wrap up that For example we could wrap and convert this in a inner class, like |
|
That makes sense. My original concern about just exposing the whole thing is that it's a mutable object, an inner class bypasses that. I'll submit an update later today. |
|
For tests, did you try running maven with the target |
|
Btw @jbentleyEG , do you still intend to push a new PR for this? |
|
@jbentleyEG @karllessard I can take this over, if necessary. |
|
Sorry, got pulled onto another task and got distracted away from this. Yes, I still plan to do a PR for this. |
|
@karllessard PR updated |
|
|
||
| public static class TensorDescription { | ||
| public final DataType dataType; | ||
| public final long[] shape; |
There was a problem hiding this comment.
Hi @jbentleyEG , looks good but as we discussed, can we return the shape as a Shape object to the users instead of a long[]?
There was a problem hiding this comment.
Hi @jbentleyEG, just checking, do you agree to make the proposed change above?
There was a problem hiding this comment.
Sorry for the flakiness, I keep getting pulled away by other tasks. I wasn't able to generate the Shape directly from the TensorShapeProto, but I was able to recreate it from the dimensions.
karllessard
left a comment
There was a problem hiding this comment.
Thanks @jbentleyEG !
Currently you can get the names of the inputs and outputs from a Signature, but there is no publicly accessible method that allows you to get the return types other than toString().
I was unable to get the tests building on my machine, but hopefully this change is small enough that it won't be an issue.