| msg53014 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2007-08-10 01:43 |
| My patch fixes small parts of the io module: * replaced all asserts with appropriate TypeError() or ValueError() exceptions * default to buffering = 1 when raw is a tty * added four methods _checkSeekable(msg=None), _checkReadable, _checkWritable and _checkClosed as convenient methods to raise exceptions when a file is closed or not read, write or seekable. * added a unit test to check __all__ * SocketIO.readable/writeable checks for self.closed, too * SocketIO.readinto/write checks for readable, writeable and closed status __all__ references SocketIO but it is not available in io.py. The definition is in socket.py and it doesn't use any socket related code directly. What do you think about moving SocketIO to io.py in the sake of keeping everything together? NOTE: You may want to check my English in the exception messages. ;) |
|
|
| msg53015 - (view) |
Author: Tony Lownds (tonylownds) |
Date: 2007-08-18 17:41 |
| Your English is fine :) The _checkSeekable etc methods are a good idea. But the message parameter is not used, so why have it? They should be lowercase names to match PEP 8. Also, shouldn't use the internal methods on parameters other than self: - assert raw.seekable() + raw._checkSeekable() I'm not sure it's a good idea to do the first todo without the second here: -XXX need to default buffer size to 1 if isatty() XXX need to support 1 meaning line-buffered |
|
|
| msg53016 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-08-19 13:47 |
| I've added the message parameter for the case that we have to use a different error message. For example: self._checkReadable("Can't read from a closed socket") I've done a lot of Zope, twisted and Qt development lately. I prefer camel case but you are right. The Python core has to follow PEP 8. What do you think about _isreadable(), _iswritable(), _isseekable()? |
|
|
| msg53017 - (view) |
Author: Tony Lownds (tonylownds) |
Date: 2007-08-19 16:49 |
| >I've added the message parameter for the case that we have to use a >different error message. For example: >self._checkReadable("Can't read from a closed socket") Ok, I can see how that would be useful in the future even if it happens not to be used in your patch -- forget my previous comment. >I've done a lot of Zope, twisted and Qt development lately. I prefer camel >case but you are right. The Python core has to follow PEP 8. What do you >think about _isreadable(), _iswritable(), _isseekable()? _is*** names sound like predicate. I think your original names in lowercase would be great: _checkreadable, _checkwritable; or with an extra underscore: _check_readable, _check_writable. Based on my previous comment: >Also, shouldn't use the internal methods on parameters other than self: >- assert raw.seekable() >+ raw._checkSeekable() ...perhaps _checkreadable, etc should be functions? |
|
|
| msg53018 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-08-20 19:04 |
| OK, I'm going to provide a new patch with _checkreadable(self, msg) etc. tomorrow. I like to keep them methods for convenient reasons. What about the SocketIO? Do you want to keep it in socket.py? |
|
|
| msg55337 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2007-08-27 17:45 |
| Thanks!! Committed revision 57564. SocketIO was recently moved to socket.py because that's the only place that uses it. So I've removed it from io.__all__. The only other change I applied was related to the isatty check -- you accidentally changed things so that buffering would *always* be set to 1 if raw.isatty is true. The intention was for this to affect the default only. |
|
|