fishy commented on code in PR #2825:
URL: https://github.com/apache/thrift/pull/2825#discussion_r1248290724


##########
compiler/cpp/src/thrift/generate/t_py_generator.cc:
##########
@@ -871,7 +871,12 @@ void 
t_py_generator::generate_py_struct_definition(ostream& out,
       }
 
       if (is_immutable(tstruct)) {
-        if (gen_newstyle_ || gen_dynamic_) {
+        if (gen_enum_ && type->is_enum()) {
+          indent(out) << "super(" << tstruct->get_name() << ", 
self).__setattr__('"

Review Comment:
   this is probably too much magic to my preference, makes me start to prefer 
the approach in #2824 again 😅 



##########
test/py/explicit_module/runtest.sh:
##########
@@ -25,10 +25,12 @@ rm -rf gen-py
 ../../../compiler/cpp/thrift --gen py test3.thrift && exit 1  # Fail since 
test3.thrift has python keywords
 ../../../compiler/cpp/thrift --gen py:enum shared_types.thrift || exit 1
 ../../../compiler/cpp/thrift --gen py:enum test4.thrift || exit 1
+../../../compiler/cpp/thrift --gen py:enum test5.thrift || exit 1

Review Comment:
   can you please also generate the code with both enum and slots (as slots 
also change the code significantly), you can use `--out` to put it under a 
different directory than `gen-py` so you can have another test around line 33 
to also make sure that the combination of enum+slots still works.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to