[Patches] [ python-Patches-1771364 ] Misc improvements for the io module

[SourceForge.net]

Patches item #1771364, was opened at 2007-08-10 03:43
Message generated for change (Comment added) made by tiran
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1771364&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Library (Lib)
Group: Python 3000
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Christian Heimes (tiran)
Assigned to: Guido van Rossum (gvanrossum)
Summary: Misc improvements for the io module

Initial Comment:
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. ;)

----------------------------------------------------------------------

>Comment By: Christian Heimes (tiran)
Date: 2007-08-20 21:04

Message:
Logged In: YES 
user_id=560817
Originator: YES

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? 

----------------------------------------------------------------------

Comment By: Tony Lownds (tonylownds)
Date: 2007-08-19 18:49

Message:
Logged In: YES 
user_id=24100
Originator: NO

>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?



----------------------------------------------------------------------

Comment By: Christian Heimes (tiran)
Date: 2007-08-19 15:47

Message:
Logged In: YES 
user_id=560817
Originator: YES

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()?

----------------------------------------------------------------------

Comment By: Tony Lownds (tonylownds)
Date: 2007-08-18 19:41

Message:
Logged In: YES 
user_id=24100
Originator: NO

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



----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1771364&group_id=5470