classification
Title: Misc improvements for the io module
Type: Stage:
Components: Library (Lib) Versions: Python 3.0
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: gvanrossum Nosy List: christian.heimes, gvanrossum, tonylownds
Priority: normal Keywords: patch

Created on 2007-08-10 01:43 by christian.heimes, last changed 2008-01-06 22:29 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
io_assert.patch christian.heimes, 2007-08-10 01:43 r56881 py3k branch
Messages (6)
msg53014 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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.
History
Date User Action Args
2008-01-06 22:29:45adminsetkeywords: - py3k
versions: + Python 3.0
2007-08-27 17:45:27gvanrossumsetstatus: open -> closed
resolution: accepted
messages: + msg55337
2007-08-10 01:43:37christian.heimescreate