fishy commented on code in PR #2825:
URL: https://github.com/apache/thrift/pull/2825#discussion_r1249004335
##########
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:
so this code:
```py
def __init__(self, whatOp=None, why=None, who=None,):
super(TestException, self).__setattr__('whatOp', whatOp)
super(TestException, self).__setattr__('why', why if hasattr(why,
'value') else shared_types.ttypes.SharedEnum.__members__.get(why))
super(TestException, self).__setattr__('who', who if hasattr(who,
'value') else TestEnum.__members__.get(who))
```
calls the parent's `__setattr__`, doesn't that essentially set those fields
on the parent instead? I guess it still "works" as when accessing them you just
inherit from the parent, but this _feels_ wrong to me.
##########
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:
yea can you add that into the PR so we make sure neither `enum` and
`enum,slots` will break in the future?
--
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]
