arrow-julia icon indicating copy to clipboard operation
arrow-julia copied to clipboard

flatbuffers split

Open ExpandingMan opened this issue 4 years ago • 2 comments

It really, really should be its own package (I assume that's the ultimate attention, but even if this is so, now we have an issue to track). And, at the risk of beating a dead horse, really I am very serious when I say the package in that form is just really, really hard to understand if something goes wrong. I just want to warn that, having had experience with this, unless that package is rewritten to make all the steps a lot more transparent, I seriously doubt that anyone other than @quinnj is going to ever wind up doing much work on it. It's not even necessarily that there's anything overly convoluted going on in reading and writing, it's just nigh on impossible to pick it apart into any kind of manageable pieces.

ExpandingMan avatar Aug 28 '20 23:08 ExpandingMan

Yes, the plan is to definitely "upstream" the flatbuffers code out of this repo back to FlatBuffers.jl. The current flatbuffers code in this repo is a direct, almost line-for-line port from the go code, which was done with the intention of not trying to reinvent the wheel and rely on their extensive testing/code coverage. Obviously, I need to do our own testing w/ this flatbuffers code, but I thought this was an easier solution that trying to fully comprehend all the various tricks/depths/nesting/complexity of flatbuffers. Now we just do exactly what they do. The biggest piece we'll need in the future for flatbuffers is getting the "compiler" working to generate the equivalent julia code from a flatbuffer type definition file. I plan on working on that, but it's a bigger project that requires a lot of C++ munging in the main monorepo, so I'm not super excited to dive right in.

Note that the "generated" flatbuffer code in this repo (for all the arrow objects), is also directly ported from the arrow go code, which also plays nicely with the flatbuffers port (same structures and everything).

I definitely agree it's not ideal; but at least wanted to walk through the steps I went through in my mind for why we are where we are. The fact is we really shouldn't rely on the current FlatBuffers.jl code at all, since it's actually pretty broken by trying to rely on this auto-conversion between julia objects & flatbuffer structure. I realized pretty quickly in the "rewrite" that the key to flatbuffers is really relying on the compiler to generate all the code for you, however verbose, and not trying to be too clever with how we do an automatic conversion w/ julia.

quinnj avatar Aug 29 '20 00:08 quinnj

I just wanted to bump this since I don't think it's been discussed in a while.

ExpandingMan avatar Oct 06 '21 21:10 ExpandingMan