fishy commented on code in PR #2816:
URL: https://github.com/apache/thrift/pull/2816#discussion_r1256493495
##########
compiler/cpp/src/thrift/generate/t_py_generator.cc:
##########
@@ -893,12 +893,42 @@ void
t_py_generator::generate_py_struct_definition(ostream& out,
if (is_immutable(tstruct)) {
out << endl;
- out << indent() << "def __setattr__(self, *args):" << endl
- << indent() << indent_str() << "raise TypeError(\"can't modify
immutable instance\")" << endl
- << endl;
- out << indent() << "def __delattr__(self, *args):" << endl
- << indent() << indent_str() << "raise TypeError(\"can't modify
immutable instance\")" << endl
- << endl;
+ out << indent() << "def __setattr__(self, *args):" << endl;
+ indent_up();
+
+ // Not user-provided fields should be editable so that the Python Standard
L`ibrary can edit
Review Comment:
```suggestion
// Not user-provided fields should be editable so that the Python
Standard Library can edit
```
##########
compiler/cpp/src/thrift/generate/t_py_generator.cc:
##########
@@ -893,12 +893,42 @@ void
t_py_generator::generate_py_struct_definition(ostream& out,
if (is_immutable(tstruct)) {
out << endl;
- out << indent() << "def __setattr__(self, *args):" << endl
- << indent() << indent_str() << "raise TypeError(\"can't modify
immutable instance\")" << endl
- << endl;
- out << indent() << "def __delattr__(self, *args):" << endl
- << indent() << indent_str() << "raise TypeError(\"can't modify
immutable instance\")" << endl
- << endl;
+ out << indent() << "def __setattr__(self, *args):" << endl;
+ indent_up();
+
+ // Not user-provided fields should be editable so that the Python Standard
L`ibrary can edit
+ // internal fields of std library base classes. For example, in Python
3.11 context managers
+ // edit the `__traceback__` field on Exceptions. Allowing this to work
with `__slots__` is
Review Comment:
```suggestion
// internal fields of std library base classes. For example, in Python
3.11 ContextManager
// edits the `__traceback__` field on Exceptions. Allowing this to work
with `__slots__` is
```
##########
compiler/cpp/src/thrift/generate/t_py_generator.cc:
##########
@@ -893,12 +893,42 @@ void
t_py_generator::generate_py_struct_definition(ostream& out,
if (is_immutable(tstruct)) {
out << endl;
- out << indent() << "def __setattr__(self, *args):" << endl
- << indent() << indent_str() << "raise TypeError(\"can't modify
immutable instance\")" << endl
- << endl;
- out << indent() << "def __delattr__(self, *args):" << endl
- << indent() << indent_str() << "raise TypeError(\"can't modify
immutable instance\")" << endl
- << endl;
+ out << indent() << "def __setattr__(self, *args):" << endl;
+ indent_up();
+
+ // Not user-provided fields should be editable so that the Python Standard
L`ibrary can edit
+ // internal fields of std library base classes. For example, in Python
3.11 context managers
+ // edit the `__traceback__` field on Exceptions. Allowing this to work
with `__slots__` is
+ // trivial because we know which fields are user-provided, without slots
we need to build a
+ // way to know which fields are user-provided.
+ if (gen_slots_ && !gen_dynamic_) {
+ out << indent() << "if args[0] not in self.__slots__:" << endl;
+ indent_up();
+ out << indent() << "super().__setattr__(*args)" << endl
+ << indent() << "return" << endl;
+ indent_down();
+ }
+ out << indent() << "raise TypeError(\"can't modify immutable instance\")"
<< endl;
+ indent_down();
+ out << endl;
+ out << indent() << "def __delattr__(self, *args):" << endl;
+ indent_up();
+
+ // Not user-provided fields should be editable so that the Python Standard
L`ibrary can edit
+ // internal fields of std library base classes. For example, in Python
3.11 context managers
+ // edit the `__traceback__` field on Exceptions. Allowing this to work
with `__slots__` is
Review Comment:
same here
--
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]
