Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Python] Python fixed size array #7529

Merged
merged 13 commits into from Sep 22, 2022

Conversation

joshua-smith8
Copy link
Contributor

Python support for fixed sized arrays of Structs and Non-Structs.

Problem:
We encountered that using python and c++ together created issues when we wanted to use fixed sized arrays in our messages.
Python seemed to only have implemented dynamic vectors relying on additional metadata in the byte arrays which when using an array from c++ wasn't included.

Solution:
To fix this we added support for these fixed arrays by modifying the length functions to retrieve the length from the original message specification files and to use this information to load the data from the byte arrays.

Testing:
We used the existing tests within py_test.py and PythonTest.sh. These now properly execute in our local tests, and we have added the generation of the array_test.fbs for this purpose.

Other:
Minor modifications were done to some of the utility functions like the snake_case conversion to handle digits correctly.

Problem:
We encountered that using fixed arrays from C++ to python that python would
not read those arrays correctly due to no size information being encoded in the byte
array itself.

Fix:
Encode the sizes within the generated python file during code generation.
Specfically we add GetArrayAsNumpy to the python version of table, which takes as input
the length of the vector. When generating the python message files we include this length
from the VectorType().fixed_length.
Problem:
When including a number in the message name we would encounter cases where SnakeCase would
not add the appropirate breaks. e.g. Int32Stamped -> int_32stamped rather than int_32_stamped.

Fix:
To fix this we can add the condition that we check if the current character is not lower and
not a digit, that we check if the previous character was a lower or digit. If it was a lower
or digit then we add the break.
Problem:
The python generated code for handling non-struct and struct vectors
and arrays was inconsistent. The calls to populate the obj api was
creating incorrect code.

Solution:
To fix this the VectorOfStruct and VectorOfNonStruct was rewritten
to handle array cases and bring the two methods in line which each
other.

Testing:
PythonTesting.sh now correctly runs and generates the code for
array_test.fbs.
Minor modifications were done on the test to use the new index
accessor for struct arrays and the script correctly sources the
location of the python code.
@github-actions github-actions bot added c++ codegen Involving generating code from schema python labels Sep 13, 2022
@joshua-smith8 joshua-smith8 changed the title Python fixed size array [Python] Python fixed size array Sep 13, 2022
@dbaileychess
Copy link
Collaborator

Can you run scripts/generate_code.py and check in the modified files? That will help me review the changes to idl_gen_python

…Struct slightly

to allow for function overloading allowing the user to get a single element of an array
or the whole array.
…to be added.

This allows us to use the GenIndents method that automatically adds new lines instead.
tests/MyGame/Example/NestedStruct.py Show resolved Hide resolved
src/util.cpp Show resolved Hide resolved
src/idl_gen_python.cpp Show resolved Hide resolved
tests/MyGame/Example/ArrayStruct.py Outdated Show resolved Hide resolved
tests/MyGame/Example/ArrayStruct.py Outdated Show resolved Hide resolved
…lthrough for GenTypePointer. Added digit check to CamelToSnake method. Added and modified tests for ToSnakeCase and CamelToSnake.
Copy link
Collaborator

@dbaileychess dbaileychess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looking good.

tests/MyGame/Example/ArrayStruct.py Outdated Show resolved Hide resolved
tests/MyGame/Example/NestedStruct.py Show resolved Hide resolved
tests/MyGame/Example/NestedStruct.py Show resolved Hide resolved
@dbaileychess dbaileychess added the release-notes The PR should be highlighted in the release notes of the next release it is in. label Sep 21, 2022
@dbaileychess
Copy link
Collaborator

Should be good, looks like you just have to sync and push again. I'll get this in tonight.

@dbaileychess dbaileychess merged commit 4131158 into google:master Sep 22, 2022
@dbaileychess
Copy link
Collaborator

Great! Thanks for your patience, I think it is a great improvement.

@joshua-smith8
Copy link
Contributor Author

not a problem, happy to contribute!

archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Sep 28, 2022
…t script [1]

(Pushed to [community-testing] for testing if the new version scheme [2] breaks anything)

[1] google/flatbuffers#7529
[2] google/flatbuffers#7547


git-svn-id: file:///srv/repos/svn-community/svn@1315604 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Sep 28, 2022
…t script [1]

(Pushed to [community-testing] for testing if the new version scheme [2] breaks anything)

[1] google/flatbuffers#7529
[2] google/flatbuffers#7547

git-svn-id: file:///srv/repos/svn-community/svn@1315604 9fca08f4-af9d-4005-b8df-a31f2cc04f65
@joshua-smith8 joshua-smith8 deleted the PythonFixedSizeArray branch October 11, 2022 10:27
dbaileychess added a commit that referenced this pull request May 17, 2023
* Don't generate types unless --python-typing specified

Fixes #7944

* Fix incorrect import statements

Fixes #7951

* Fix $PYTHONPATH in PythonTest.sh

Regressed from #7529

* PythonTest: fail if something goes wrong

GitHub Actions runs `bash PythonTest.sh`, and thus failures were not
visible.

* Build flatc for Python tests

* Regenerate codes

---------

Co-authored-by: Derek Bailey <derekbailey@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ codegen Involving generating code from schema python release-notes The PR should be highlighted in the release notes of the next release it is in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants