flatbuffers icon indicating copy to clipboard operation
flatbuffers copied to clipboard

Resolve #4530: Added vector union support for Python

Open surculus12 opened this issue 3 years ago • 4 comments

Adds vector union support for Python.

Tests are consistent with the tests I saw for vector unions in Java, except that I added more than one Attacker struct to the vector.

Note that I adjust the offset of the vector by the table position after finding the offset of the vector because the table's position is added in both the vector and union methods, and would otherwise be off by the position. Doing it in another way would require modifying the python package. For example, in the Java library the table position seems to be added when calling the vector/union methods, instead of inside the methods themselves. This allows the position to be added only on the vector call, the result of which is simply passed to the union method.

(Note: I closed a previous PR while rebasing for the CLA.)

surculus12 avatar Jun 27 '22 16:06 surculus12

Thanks for the contribution! Note that AdvancedUnionFeatures includes more than just unions in vectors. Please consider extending support to the other advanced features too

CasperN avatar Jun 28 '22 15:06 CasperN

I'll add the other tests later in the week.

Could you clarify which features fall under AdvancedUnionFeatures? This was the only one I needed for my project, but I'd be happy to look into related features. EDIT: I missed your first comment. I do, actually, need those features too, so I'll look into adding those. Should I close the PR until then or keep it open?

surculus12 avatar Jun 28 '22 23:06 surculus12

Should I close the PR until then or keep it open?

Feel free to leave this PR open

CasperN avatar Jun 29 '22 13:06 CasperN

@CasperN Got sidetracked with a different project, but here is the update. Strings and structs should now work (the latter worked out of the box).

Issue with strings: Because of the way the table comes out of the union helper method (in this case, Movie.Characters) we are left with a table that points directly at the string location (len + string) and not the offset of the string. Since the Table.String method takes an offset to an offset of the string, some solution needs to work around this. I created a method in the Python library Table object called UnionString which treats the offset as the real offset, not the offset of the offset. However, we could do something else if that is not in line with the project's standards.

surculus12 avatar Jul 19 '22 15:07 surculus12

@surculus12 What's the status on this?

dbaileychess avatar Aug 06 '22 05:08 dbaileychess

@dbaileychess @surculus12 Hi, what's the status on this?

misha-antonenko avatar Dec 12 '23 18:12 misha-antonenko