Skip to content

Fix Python object API packing with no type prefixes #9013

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.

Sign up for GitHub

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

Open
steadytao wants to merge 1 commit into
base: master
Choose a base branch
from
+80 −29
Open

Fix Python object API packing with no type prefixes #9013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 50 additions & 29 deletions src/idl_gen_python.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2320,21 +2320,48 @@ class PythonGenerator : public BaseGenerator {
code += ")\n";
}

std::string TableStartFunctionName(const StructDef& struct_def) const {
return parser_.opts.python_no_type_prefix_suffix
? "Start"
: namer_.Type(struct_def) + "Start";
}

std::string TableAddFunctionName(const StructDef& struct_def,
const FieldDef& field) const {
return parser_.opts.python_no_type_prefix_suffix
? "Add" + namer_.Method(field)
: namer_.Type(struct_def) + "Add" + namer_.Method(field);
}

std::string TableStartVectorFunctionName(const StructDef& struct_def,
const FieldDef& field) const {
return parser_.opts.python_no_type_prefix_suffix
? "Start" + namer_.Method(field) + "Vector"
: namer_.Type(struct_def) + "Start" + namer_.Method(field) +
"Vector";
}

std::string TableEndFunctionName(const StructDef& struct_def) const {
return parser_.opts.python_no_type_prefix_suffix
? "End"
: namer_.Type(struct_def) + "End";
}

void GenPackForStructVectorField(const StructDef& struct_def,
const FieldDef& field,
std::string* code_prefix_ptr,
std::string* code_ptr) const {
auto& code_prefix = *code_prefix_ptr;
auto& code = *code_ptr;
const auto field_field = namer_.Field(field);
const auto struct_type = namer_.Type(struct_def);
const auto field_method = namer_.Method(field);
const auto start_vector = TableStartVectorFunctionName(struct_def, field);
const auto add_field = TableAddFunctionName(struct_def, field);

// Creates the field.
code_prefix += GenIndents(2) + "if self." + field_field + " is not None:";
if (field.value.type.struct_def->fixed) {
code_prefix += GenIndents(3) + struct_type + "Start" + field_method +
"Vector(builder, len(self." + field_field + "))";
code_prefix += GenIndents(3) + start_vector + "(builder, len(self." +
field_field + "))";
code_prefix += GenIndents(3) + "for i in reversed(range(len(self." +
field_field + "))):";
code_prefix +=
Expand All @@ -2349,8 +2376,8 @@ class PythonGenerator : public BaseGenerator {
code_prefix += GenIndents(4) + field_field + "list.append(self." +
field_field + "[i].Pack(builder))";

code_prefix += GenIndents(3) + struct_type + "Start" + field_method +
"Vector(builder, len(self." + field_field + "))";
code_prefix += GenIndents(3) + start_vector + "(builder, len(self." +
field_field + "))";
code_prefix += GenIndents(3) + "for i in reversed(range(len(self." +
field_field + "))):";
code_prefix += GenIndents(4) + "builder.PrependUOffsetTRelative" + "(" +
Expand All @@ -2360,8 +2387,7 @@ class PythonGenerator : public BaseGenerator {

// Adds the field into the struct.
code += GenIndents(2) + "if self." + field_field + " is not None:";
code += GenIndents(3) + struct_type + "Add" + field_method + "(builder, " +
field_field + ")";
code += GenIndents(3) + add_field + "(builder, " + field_field + ")";
}

void GenPackForScalarVectorFieldHelper(const StructDef& struct_def,
Expand All @@ -2370,12 +2396,11 @@ class PythonGenerator : public BaseGenerator {
int indents) const {
auto& code = *code_ptr;
const auto field_field = namer_.Field(field);
const auto field_method = namer_.Method(field);
const auto struct_type = namer_.Type(struct_def);
const auto start_vector = TableStartVectorFunctionName(struct_def, field);
const auto vectortype = field.value.type.VectorType();

code += GenIndents(indents) + struct_type + "Start" + field_method +
"Vector(builder, len(self." + field_field + "))";
code += GenIndents(indents) + start_vector + "(builder, len(self." +
field_field + "))";
code += GenIndents(indents) + "for i in reversed(range(len(self." +
field_field + "))):";
code += GenIndents(indents + 1) + "builder.Prepend";
Expand Down Expand Up @@ -2433,12 +2458,11 @@ class PythonGenerator : public BaseGenerator {
auto& code_prefix = *code_prefix_ptr;
const auto field_field = namer_.Field(field);
const auto field_method = namer_.Method(field);
const auto struct_type = namer_.Type(struct_def);
const auto add_field = TableAddFunctionName(struct_def, field);

// Adds the field into the struct.
code += GenIndents(2) + "if self." + field_field + " is not None:";
code += GenIndents(3) + struct_type + "Add" + field_method + "(builder, " +
field_field + ")";
code += GenIndents(3) + add_field + "(builder, " + field_field + ")";

// Creates the field.
code_prefix += GenIndents(2) + "if self." + field_field + " is not None:";
Expand Down Expand Up @@ -2481,8 +2505,7 @@ class PythonGenerator : public BaseGenerator {
auto& code_prefix = *code_prefix_ptr;
auto& code = *code_ptr;
const auto field_field = namer_.Field(field);
const auto field_method = namer_.Method(field);
const auto struct_type = namer_.Type(struct_def);
const auto add_field = TableAddFunctionName(struct_def, field);

if (field.value.type.struct_def->fixed) {
// Pure struct fields need to be created along with their parent
Expand All @@ -2498,8 +2521,7 @@ class PythonGenerator : public BaseGenerator {
code += GenIndents(2) + "if self." + field_field + " is not None:";
}

code += GenIndents(3) + struct_type + "Add" + field_method + "(builder, " +
field_field + ")";
code += GenIndents(3) + add_field + "(builder, " + field_field + ")";
}

void GenPackForUnionField(const StructDef& struct_def, const FieldDef& field,
Expand All @@ -2508,28 +2530,27 @@ class PythonGenerator : public BaseGenerator {
auto& code_prefix = *code_prefix_ptr;
auto& code = *code_ptr;
const auto field_field = namer_.Field(field);
const auto field_method = namer_.Method(field);
const auto struct_type = namer_.Type(struct_def);
const auto add_field = TableAddFunctionName(struct_def, field);

// TODO(luwa): TypeT should be moved under the None check as well.
code_prefix += GenIndents(2) + "if self." + field_field + " is not None:";
code_prefix += GenIndents(3) + field_field + " = self." + field_field +
".Pack(builder)";
code += GenIndents(2) + "if self." + field_field + " is not None:";
code += GenIndents(3) + struct_type + "Add" + field_method + "(builder, " +
field_field + ")";
code += GenIndents(3) + add_field + "(builder, " + field_field + ")";
}

void GenPackForTable(const StructDef& struct_def,
std::string* code_ptr) const {
auto& code_base = *code_ptr;
std::string code, code_prefix;
const auto struct_var = namer_.Variable(struct_def);
const auto struct_type = namer_.Type(struct_def);
const auto start_table = TableStartFunctionName(struct_def);
const auto end_table = TableEndFunctionName(struct_def);

GenReceiverForObjectAPI(struct_def, code_ptr);
code_base += "Pack(self, builder):";
code += GenIndents(2) + struct_type + "Start(builder)";
code += GenIndents(2) + start_table + "(builder)";
for (auto it = struct_def.fields.vec.begin();
it != struct_def.fields.vec.end(); ++it) {
auto& field = **it;
Expand Down Expand Up @@ -2558,26 +2579,26 @@ class PythonGenerator : public BaseGenerator {
break;
}
case BASE_TYPE_STRING: {
const auto add_field = TableAddFunctionName(struct_def, field);
code_prefix +=
GenIndents(2) + "if self." + field_field + " is not None:";
code_prefix += GenIndents(3) + field_field +
" = builder.CreateString(self." + field_field + ")";
code += GenIndents(2) + "if self." + field_field + " is not None:";
code += GenIndents(3) + struct_type + "Add" + field_method +
"(builder, " + field_field + ")";
code += GenIndents(3) + add_field + "(builder, " + field_field + ")";
break;
}
default:
// Generates code for scalar values. If the value equals to the
// default value, builder will automatically ignore it. So we don't
// need to check the value ahead.
code += GenIndents(2) + struct_type + "Add" + field_method +
code += GenIndents(2) + TableAddFunctionName(struct_def, field) +
"(builder, self." + field_field + ")";
break;
}
}

code += GenIndents(2) + struct_var + " = " + struct_type + "End(builder)";
code += GenIndents(2) + struct_var + " = " + end_table + "(builder)";
code += GenIndents(2) + "return " + struct_var;

code_base += code_prefix + code;
Expand Down
1 change: 1 addition & 0 deletions tests/PythonTest.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ ${test_dir}/../flatc -p -o ${gen_code_path} -I include_test arrays_test.fbs --ge
${test_dir}/../flatc -p -o ${gen_code_path} -I include_test nested_union_test.fbs --gen-object-api --python-typing --python-decode-obj-api-strings
${test_dir}/../flatc -p -o ${gen_code_path} -I include_test service_test.fbs --grpc --grpc-python-typed-handlers --python-typing --no-python-gen-numpy --gen-onefile
${test_dir}/../flatc -p -o ${gen_code_path} union_name_test.fbs --gen-object-api
${test_dir}/../flatc -p -o ${gen_code_path} python_no_type_prefix_suffix_test.fbs --gen-object-api --python-no-type-prefix-suffix

# Syntax: run_tests <interpreter> <benchmark vtable dedupes>
# <benchmark read count> <benchmark build count>
Expand Down
19 changes: 19 additions & 0 deletions tests/py_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
import monster_test_generated # the one-file version
import optional_scalars
import optional_scalars.ScalarStuff
import TableBoo
import TableFoo
import union_name_test.Container # refers to generated code
import union_name_test.Foo # refers to generated code
import union_name_test.Bar # refers to generated code
Expand Down Expand Up @@ -340,6 +342,23 @@ def test_optional_scalars_with_pack_and_unpack(self):
self.assertTrue(optsT2.maybeEnum is None)
self.assertTrue(optsT2.defaultU64 == 42)

def test_python_no_type_prefix_suffix_with_pack(self):
fooT = TableFoo.TableFooT()
fooT.field1 = 7
fooT.field2 = 'hello'

booT = TableBoo.TableBooT()
booT.table1 = fooT

builder = flatbuffers.Builder(0)
builder.Finish(booT.Pack(builder))

boo = TableBoo.TableBoo.GetRootAs(builder.Bytes, builder.Head())
foo = boo.Table1()

self.assertEqual(foo.Field1(), 7)
self.assertEqual(foo.Field2().decode('utf-8'), 'hello')


class TestAllMutableCodePathsOfExampleSchema(unittest.TestCase):
"""Tests the object API generated for monster_test.fbs for mutation
Expand Down
10 changes: 10 additions & 0 deletions tests/python_no_type_prefix_suffix_test.fbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
table TableFoo {
field1:uint8;
field2:string;
}

table TableBoo {
table1:TableFoo;
}

root_type TableBoo;