rust icon indicating copy to clipboard operation
rust copied to clipboard

Adding AttrValue enum

Open whereistejas opened this issue 4 years ago • 1 comments

Hi @adamcrume,

I have opened this PR. I thought we could move our discussion from the issue page to here.

TODO: Struct definitions for AttrValue::list and AttrValue::func are still pending

Resolves issue: #290

Signed-off-by: Tejas Sanap [email protected]

whereistejas avatar Jan 26 '21 17:01 whereistejas

I have a couple of questions.

  1. In the struct NameAttrList there is a field called attr. We can see that it's type is: https://github.com/tensorflow/rust/blob/ceefb76ad8aa7b7e2fe0724279fead86ffe35539/src/protos/attr_value.rs#L1377-L1385 The type of the hashmap's value itself is: https://github.com/tensorflow/rust/blob/ceefb76ad8aa7b7e2fe0724279fead86ffe35539/src/protos/attr_value.rs#L26-L33 Do we have an equivalent of the AttrValue in protos/attr_value.rs in src/lib.rs the way we have for shape? I couldn't find any.
  2. In src/protos/attr_value.rs we have a struct called AttrValue which uses the enum AttrValue_oneof_value. Why are we not creating the same objects in lib.rs instead of changing their names? I'm just trying to understand your reasoning and what end goal you have in mind.
  3. I'm curious about how the enums and structs we are creating fits into the bigger picture. Why are we exactly adding this AttrValue enum in lib.rs? What will it enable us to do? What problems will it solve? I know this question sounds kind of stupid, but I want to understand the functional side of the code we are writing.

whereistejas avatar Jan 26 '21 18:01 whereistejas