[Twisted-Python] Old-class-only support in manhole
Hello! Creating a test case for a change in twisted/manhole/explorer.py, I found these lines in CRUFT_WatchyThingie.watchObject: if type(object) is not types.InstanceType: raise TypeError, "Sorry, can only place a watch on Instances." Always talking in Python 2, if you pass an instance of an old style class as the "object", it will be ok; but if you pass a new style class, it will raise TypeError. So, I wanted just to ask if this is on purpose, should I open a bug about it, or what. Thank you! -- . Facundo Blog: http://www.taniquetil.com.ar/plog/ PyAr: http://www.python.org/ar/
On 25 Mar, 10:26 pm, facundobatista@gmail.com wrote:
Hello!
Creating a test case for a change in twisted/manhole/explorer.py, I found these lines in CRUFT_WatchyThingie.watchObject:
if type(object) is not types.InstanceType: raise TypeError, "Sorry, can only place a watch on Instances."
Always talking in Python 2, if you pass an instance of an old style class as the "object", it will be ok; but if you pass a new style class, it will raise TypeError.
So, I wanted just to ask if this is on purpose, should I open a bug about it, or what.
This is really old, untested code. At the time it was written, there was no such thing as a new-style class. No one bothered to update the code when new-style classes were introduced into the language. And the last time I tried to actually use this functionality (at least two years ago), it didn't work anymore anyway (even for classic classes). The idea here is cool, and I think it would be nice to support it, but this code is pretty gross and, as far as I know, broken. I think we should think about getting rid of it. If we go that route, then you should be able to ignore it for your Python 3 porting efforts. Jean-Paul
On Sat, Mar 26, 2011 at 3:13 PM, <exarkun@twistedmatrix.com> wrote:
The idea here is cool, and I think it would be nice to support it, but this code is pretty gross and, as far as I know, broken. I think we should think about getting rid of it. If we go that route, then you should be able to ignore it for your Python 3 porting efforts.
However, I added a couple of tests for that code, just to test the line I changed from 'new' to 'types' module (it was requested by the reviewer in Trac). -- . Facundo Blog: http://www.taniquetil.com.ar/plog/ PyAr: http://www.python.org/ar/
On Sat, Mar 26, 2011 at 2:47 PM, Facundo Batista <facundobatista@gmail.com>wrote:
On Sat, Mar 26, 2011 at 3:13 PM, <exarkun@twistedmatrix.com> wrote:
The idea here is cool, and I think it would be nice to support it, but this code is pretty gross and, as far as I know, broken. I think we should think about getting rid of it. If we go that route, then you should be able to ignore it for your Python 3 porting efforts.
However, I added a couple of tests for that code, just to test the line I changed from 'new' to 'types' module (it was requested by the reviewer in Trac).
It would definitely be nicer to make it work than to remove it. You should just feel free to consider the option of removing it if making it function correctly is an unreasonable amount of work.
On 04:23 am, glyph@twistedmatrix.com wrote:
On Sat, Mar 26, 2011 at 2:47 PM, Facundo Batista <facundobatista@gmail.com>wrote:
On Sat, Mar 26, 2011 at 3:13 PM, <exarkun@twistedmatrix.com> wrote:
The idea here is cool, and I think it would be nice to support it, but this code is pretty gross and, as far as I know, broken. I think we should think about getting rid of it. If we go that route, then you should be able to ignore it for your Python 3 porting efforts.
However, I added a couple of tests for that code, just to test the line I changed from 'new' to 'types' module (it was requested by the reviewer in Trac).
It would definitely be nicer to make it work than to remove it. You should just feel free to consider the option of removing it if making it function correctly is an unreasonable amount of work.
I don't know. Skim http://twistedmatrix.com/trac/browser/trunk/twisted/manhole/explorer.py?rev=... and see if you think it's actually fixable. :) In any case, Facundo did write tests that cover the lines he needed to change for that ticket, and I applied his patch to trunk. Still, as far as I know explorer doesn't actually work, and most of explorer.py is still untested. Jean-Paul
participants (3)
-
exarkun＠twistedmatrix.com -
Facundo Batista -
Glyph Lefkowitz