FlatBuffers.jl icon indicating copy to clipboard operation
FlatBuffers.jl copied to clipboard

support for optional fields?

Open ExpandingMan opened this issue 6 years ago • 3 comments
trafficstars

I'm not sure I really understand what's going on here yet, so forgive me if this is a stupid question, but it seems to me that FlatBuffer fields are always optional, however this package does not seem to support that as the constructors it defines to not seem to allow undefined fields.

Is this a missing feature, or am I just missing something here?

ExpandingMan avatar Mar 17 '19 01:03 ExpandingMan

Sorry, I now know somewhat better what I'm talking about and I now know what my issues were.

So yes, it appears flatbuffers wants you to allow for the possibility of all fields being optional. What "optional" means in the corresponding Julia struct depends somewhat on what the field is.

Anyway, this brings me to a bigger question. Right now the Julia convenience macros are missing a few features such as setting some reasonable default field values and two of the default constructors. I would say that, at the very least, if we do not anticipate the flatc PR getting merged any time soon, it would be really nice if we could have better helper macros in this package. Such macros would potentially be quite useful even with flatc, as it could potentially generate simpler looking code, though in that case we'd be sacrificing some explicitness.

What are everyone's thoughts on having a few additional macros which generate the above? I'm currently working on overhauling Arrow, so I might make sense for me to make a PR for this so that I could use it there.

ExpandingMan avatar Mar 17 '19 20:03 ExpandingMan

Glad to hear any suggestions and happy to review PRs. I also left a comment in the flatc pull request about what's left to be done there.

rjkat avatar Mar 17 '19 23:03 rjkat

Cool thanks.

If I have a specific plan I'll post it here. Whether I take this on will depend on how much I feel I need it for Arrow.jl.

ExpandingMan avatar Mar 17 '19 23:03 ExpandingMan