You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "Jonathan Ellis (JIRA)" <ji...@apache.org> on 2008/12/24 17:54:44 UTC
[jira] Created: (THRIFT-241) Python __repr__ is confusing and does
not eval to the object in question
Python __repr__ is confusing and does not eval to the object in question
------------------------------------------------------------------------
Key: THRIFT-241
URL: https://issues.apache.org/jira/browse/THRIFT-241
Project: Thrift
Issue Type: Improvement
Components: Compiler (Python)
Affects Versions: 0.1
Reporter: Jonathan Ellis
Having the repr return the repr of __dict__ is confusing, because the object is not a dict and does not act (much) like one. 100% of the 3 python developers I have seen who are new to thrift were confused at first why the repr looked like a dict but obj[attributename] raised KeyError. Additionally, this violates the repr guideline that where possible eval(repr(obj)) == obj.
Finally, specifying a __str__ equal to __repr__ is redundant, since str() will use __repr__ if no __str__ is given.
Here is a patch:
$ diff -u compiler/cpp/src/generate/t_py_generator.cc.old compiler/cpp/sr\
c/generate/t_py_generator.cc
--- compiler/cpp/src/generate/t_py_generator.cc.old 2008-12-24 16:36:54.000000000 +0000
+++ compiler/cpp/src/generate/t_py_generator.cc 2008-12-24 16:49:54.000000000 +0000
@@ -614,11 +614,10 @@
// Printing utilities so that on the command line thrift
// structs look pretty like dictionaries
out <<
- indent() << "def __str__(self):" << endl <<
- indent() << " return str(self.__dict__)" << endl <<
- endl <<
indent() << "def __repr__(self):" << endl <<
- indent() << " return repr(self.__dict__)" << endl <<
+ indent() << " L = ['%s=%r' % (key, value)" << endl <<
+ indent() << " for key, value in self.__dict__.iteritems()]" << endl <<
+ indent() << " return '%s(%s)' % (self.__class__.__name__, ', '.join(L))" << endl <<
endl;
// Equality and inequality methods that compare by value
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (THRIFT-241) Python __repr__ is confusing and does
not eval to the object in question
Posted by "Mark Slee (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-241?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mark Slee closed THRIFT-241.
----------------------------
Was committed as r731685
> Python __repr__ is confusing and does not eval to the object in question
> ------------------------------------------------------------------------
>
> Key: THRIFT-241
> URL: https://issues.apache.org/jira/browse/THRIFT-241
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Affects Versions: 0.1
> Reporter: Jonathan Ellis
> Attachments: repr.patch
>
>
> Having the repr return the repr of __dict__ is confusing, because the object is not a dict and does not act (much) like one. 100% of the 3 python developers I have seen who are new to thrift were confused at first why the repr looked like a dict but obj[attributename] raised KeyError. Additionally, this violates the repr guideline that where possible eval(repr(obj)) == obj.
> Finally, specifying a __str__ equal to __repr__ is redundant, since str() will use __repr__ if no __str__ is given.
> Here is a patch:
> $ diff -u compiler/cpp/src/generate/t_py_generator.cc.old compiler/cpp/sr\
> c/generate/t_py_generator.cc
> --- compiler/cpp/src/generate/t_py_generator.cc.old 2008-12-24 16:36:54.000000000 +0000
> +++ compiler/cpp/src/generate/t_py_generator.cc 2008-12-24 16:49:54.000000000 +0000
> @@ -614,11 +614,10 @@
> // Printing utilities so that on the command line thrift
> // structs look pretty like dictionaries
> out <<
> - indent() << "def __str__(self):" << endl <<
> - indent() << " return str(self.__dict__)" << endl <<
> - endl <<
> indent() << "def __repr__(self):" << endl <<
> - indent() << " return repr(self.__dict__)" << endl <<
> + indent() << " L = ['%s=%r' % (key, value)" << endl <<
> + indent() << " for key, value in self.__dict__.iteritems()]" << endl <<
> + indent() << " return '%s(%s)' % (self.__class__.__name__, ', '.join(L))" << endl <<
> endl;
> // Equality and inequality methods that compare by value
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (THRIFT-241) Python __repr__ is confusing and
does not eval to the object in question
Posted by "Esteve Fernandez (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-241?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12659352#action_12659352 ]
Esteve Fernandez commented on THRIFT-241:
-----------------------------------------
+1 The patch looks good.
> Python __repr__ is confusing and does not eval to the object in question
> ------------------------------------------------------------------------
>
> Key: THRIFT-241
> URL: https://issues.apache.org/jira/browse/THRIFT-241
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Affects Versions: 0.1
> Reporter: Jonathan Ellis
> Attachments: repr.patch
>
>
> Having the repr return the repr of __dict__ is confusing, because the object is not a dict and does not act (much) like one. 100% of the 3 python developers I have seen who are new to thrift were confused at first why the repr looked like a dict but obj[attributename] raised KeyError. Additionally, this violates the repr guideline that where possible eval(repr(obj)) == obj.
> Finally, specifying a __str__ equal to __repr__ is redundant, since str() will use __repr__ if no __str__ is given.
> Here is a patch:
> $ diff -u compiler/cpp/src/generate/t_py_generator.cc.old compiler/cpp/sr\
> c/generate/t_py_generator.cc
> --- compiler/cpp/src/generate/t_py_generator.cc.old 2008-12-24 16:36:54.000000000 +0000
> +++ compiler/cpp/src/generate/t_py_generator.cc 2008-12-24 16:49:54.000000000 +0000
> @@ -614,11 +614,10 @@
> // Printing utilities so that on the command line thrift
> // structs look pretty like dictionaries
> out <<
> - indent() << "def __str__(self):" << endl <<
> - indent() << " return str(self.__dict__)" << endl <<
> - endl <<
> indent() << "def __repr__(self):" << endl <<
> - indent() << " return repr(self.__dict__)" << endl <<
> + indent() << " L = ['%s=%r' % (key, value)" << endl <<
> + indent() << " for key, value in self.__dict__.iteritems()]" << endl <<
> + indent() << " return '%s(%s)' % (self.__class__.__name__, ', '.join(L))" << endl <<
> endl;
> // Equality and inequality methods that compare by value
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (THRIFT-241) Python __repr__ is confusing and does
not eval to the object in question
Posted by "David Reiss (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-241?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
David Reiss resolved THRIFT-241.
--------------------------------
Resolution: Fixed
> Python __repr__ is confusing and does not eval to the object in question
> ------------------------------------------------------------------------
>
> Key: THRIFT-241
> URL: https://issues.apache.org/jira/browse/THRIFT-241
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Affects Versions: 0.1
> Reporter: Jonathan Ellis
> Attachments: repr.patch
>
>
> Having the repr return the repr of __dict__ is confusing, because the object is not a dict and does not act (much) like one. 100% of the 3 python developers I have seen who are new to thrift were confused at first why the repr looked like a dict but obj[attributename] raised KeyError. Additionally, this violates the repr guideline that where possible eval(repr(obj)) == obj.
> Finally, specifying a __str__ equal to __repr__ is redundant, since str() will use __repr__ if no __str__ is given.
> Here is a patch:
> $ diff -u compiler/cpp/src/generate/t_py_generator.cc.old compiler/cpp/sr\
> c/generate/t_py_generator.cc
> --- compiler/cpp/src/generate/t_py_generator.cc.old 2008-12-24 16:36:54.000000000 +0000
> +++ compiler/cpp/src/generate/t_py_generator.cc 2008-12-24 16:49:54.000000000 +0000
> @@ -614,11 +614,10 @@
> // Printing utilities so that on the command line thrift
> // structs look pretty like dictionaries
> out <<
> - indent() << "def __str__(self):" << endl <<
> - indent() << " return str(self.__dict__)" << endl <<
> - endl <<
> indent() << "def __repr__(self):" << endl <<
> - indent() << " return repr(self.__dict__)" << endl <<
> + indent() << " L = ['%s=%r' % (key, value)" << endl <<
> + indent() << " for key, value in self.__dict__.iteritems()]" << endl <<
> + indent() << " return '%s(%s)' % (self.__class__.__name__, ', '.join(L))" << endl <<
> endl;
> // Equality and inequality methods that compare by value
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (THRIFT-241) Python __repr__ is confusing and does
not eval to the object in question
Posted by "Jonathan Ellis (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/THRIFT-241?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis updated THRIFT-241:
----------------------------------
Attachment: repr.patch
Sorry about the messed up formatting. Clean patch attached.
> Python __repr__ is confusing and does not eval to the object in question
> ------------------------------------------------------------------------
>
> Key: THRIFT-241
> URL: https://issues.apache.org/jira/browse/THRIFT-241
> Project: Thrift
> Issue Type: Improvement
> Components: Compiler (Python)
> Affects Versions: 0.1
> Reporter: Jonathan Ellis
> Attachments: repr.patch
>
>
> Having the repr return the repr of __dict__ is confusing, because the object is not a dict and does not act (much) like one. 100% of the 3 python developers I have seen who are new to thrift were confused at first why the repr looked like a dict but obj[attributename] raised KeyError. Additionally, this violates the repr guideline that where possible eval(repr(obj)) == obj.
> Finally, specifying a __str__ equal to __repr__ is redundant, since str() will use __repr__ if no __str__ is given.
> Here is a patch:
> $ diff -u compiler/cpp/src/generate/t_py_generator.cc.old compiler/cpp/sr\
> c/generate/t_py_generator.cc
> --- compiler/cpp/src/generate/t_py_generator.cc.old 2008-12-24 16:36:54.000000000 +0000
> +++ compiler/cpp/src/generate/t_py_generator.cc 2008-12-24 16:49:54.000000000 +0000
> @@ -614,11 +614,10 @@
> // Printing utilities so that on the command line thrift
> // structs look pretty like dictionaries
> out <<
> - indent() << "def __str__(self):" << endl <<
> - indent() << " return str(self.__dict__)" << endl <<
> - endl <<
> indent() << "def __repr__(self):" << endl <<
> - indent() << " return repr(self.__dict__)" << endl <<
> + indent() << " L = ['%s=%r' % (key, value)" << endl <<
> + indent() << " for key, value in self.__dict__.iteritems()]" << endl <<
> + indent() << " return '%s(%s)' % (self.__class__.__name__, ', '.join(L))" << endl <<
> endl;
> // Equality and inequality methods that compare by value
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.