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

23.5.8's Python code generator creates import statements that break existing projects using 23.3.3 or earlier #7951

Closed
stewartmiles opened this issue May 10, 2023 · 3 comments

Comments

@stewartmiles
Copy link
Contributor

#7858 changes the way Python imports are generated from, for example:

from reflection.EnumVal import EnumVal

to:

from .reflection.EnumVal import EnumVal

This breaks packages where the root is the generated flatbuffers package name. For example, we have code that generates Python FlatBuffers code from TensorFlow Lite's schema that places all code into a top-level Python package called tflite i.e given the file structure:

tensorflow_lite_model_schema/pyproject.toml
tensorflow_lite_model_schema/setup.py
tensorflow_lite_model_schema/tflite/  # Generated FlatBuffers code.
tensorflow_lite_model_schema/tflite_model_schema/ # mirror of https://github.com/tensorflow/tensorflow/blob/master/tensorflow/lite/schema

Where the code is generated using a command line like this:

cd tensorflow_lite_model_schema
flatc --python -o . --gen-object-api -I . tflite_model_schema/schema.fbs

This yields a load of generated files:

ls tensorflow_lite_model_schema/tflite | sort | head -n 5
AbsOptions.py
ActivationFunctionType.py
AddNOptions.py
AddOptions.py
ArgMaxOptions.py

Finally, when importing one of these modules that transitively imports another module the problem pops up:

python -c '
import sys;
sys.path.append(".");
import flatbuffers;
from tflite import Model;
from tflite import OperatorCode;
builder = flatbuffers.builder.Builder();
model_object = Model.ModelT();
model_object.operatorCodes = [OperatorCode.OperatorCodeT()];
builder.Finish(model_object.Pack(builder));
model_flatbuffer = builder.Output();
model = Model.Model.GetRootAs(model_flatbuffer); print(model.OperatorCodes(0))'

produces:

Traceback (most recent call last):
  File "<string>", line 12, in <module>
  File "/home/stewart/src/agentic/service_clean/tensorflow_lite_model_schema/tflite/Model.py", line 45, in OperatorCodes
    from .tflite.OperatorCode import OperatorCode
ModuleNotFoundError: No module named 'tflite.tflite'

@maxburke and @dbaileychess thoughts on a fix?

@dbaileychess
Copy link
Collaborator

I'll defer to @maxburke since I don't really do python much, but it seems like this line https://github.com/google/flatbuffers/blob/master/src/idl_gen_python.cpp#L278 and others like it are the issue?

I think the lack of --python-typings should produce identical code to what it was doing.

@stewartmiles could you remove those instances of the "." + in idl_gen_python.cpp and see if that is the sole cause?

@stewartmiles
Copy link
Contributor Author

@dbaileychess I ended up doing a local workaround which is pretty similar. I patched the generated code to replace from .foo import Bar with from foo import Bar and things superficially seem to be working. I'll let you know if we come across any other issues as we run more extensive tests.

@yan12125
Copy link
Contributor

could you remove those instances of the "." + in idl_gen_python.cpp and see if that is the sole cause?

I can get ./tests/PythonTest.sh pass with these changes and other stuffs mentioned in #7944:

diff --git a/src/idl_gen_python.cpp b/src/idl_gen_python.cpp
index 72b54604..ff535d15 100644
--- a/src/idl_gen_python.cpp
+++ b/src/idl_gen_python.cpp
@@ -275,7 +275,7 @@ class PythonGenerator : public BaseGenerator {
     code += namer_.Method(field);
 
     const ImportMapEntry import_entry = {
-      "." + GenPackageReference(field.value.type), TypeName(field)
+      GenPackageReference(field.value.type), TypeName(field)
     };
 
     if (parser_.opts.python_typing) {
@@ -337,7 +337,7 @@ class PythonGenerator : public BaseGenerator {
     code += namer_.Method(field) + "(self)";
 
     const ImportMapEntry import_entry = {
-      "." + GenPackageReference(field.value.type), TypeName(field)
+      GenPackageReference(field.value.type), TypeName(field)
     };
 
     if (parser_.opts.python_typing) {
@@ -446,7 +446,7 @@ class PythonGenerator : public BaseGenerator {
     GenReceiver(struct_def, code_ptr);
     code += namer_.Method(field);
     const ImportMapEntry import_entry = {
-      "." + GenPackageReference(field.value.type), TypeName(field)
+      GenPackageReference(field.value.type), TypeName(field)
     };
 
     if (parser_.opts.python_typing) {
@@ -570,7 +570,7 @@ class PythonGenerator : public BaseGenerator {
     std::string qualified_name = NestedFlatbufferType(unqualified_name);
     if (qualified_name.empty()) { qualified_name = nested->constant; }
 
-    const ImportMapEntry import_entry = { "." + qualified_name,
+    const ImportMapEntry import_entry = { qualified_name,
                                           unqualified_name };
 
     auto &code = *code_ptr;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants