Re: [Python-Dev] Possible py3k io wierdness

2009-04-12 Thread Brian Quinlan

I've added a new proposed patch to:
http://bugs.python.org/issue5700

The idea is:
- only IOBase implements close() (though a subclass can override close
  without causing problems so long as it calls super().close() or
  calls .flush() and ._close() directly)
- change IOBase.close to call .flush() and then ._close()
- .flush() invokes super().flush() in every class except IOBase
- ._close() invokes super()._close() in every class except IOBase
- FileIO is implemented in Python in _pyio.py so that it can have the
  same base class as the other Python-implemented files classes
- tests verify that .flush() is not called after the file is closed
- tests verify that ._close()/.flush() calls are propagated correctly

On nice side effect is that inheritance is a lot easier and MI works as 
expected i.e.


class DebugClass(IOBase):
  def flush(self):
print(some debug info)
super().flush()
  def _close(self):
print(some debug info
super()._close()

class MyClass(FileIO, DebugClass): # whatever order makes sense
  ...

m = MyClass(...)
m.close()
# Will call:
#   IOBase.close()
#   DebugClass.flush()  # FileIO has no .flush method
#   IOBase.flush()
#   FileIO._close()
#   DebugClass._close()
#   IOBase._close()


Cheers,
Brian
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-06 Thread Nick Coghlan
Alex Martelli wrote:
 Queue.Queue in 2.* (and queue.Queue in 3.*) is like that too -- the
 single leading underscore meaning protected (I'm here for subclasses
 to override me, only in C++ parlance) and a great way to denote hook
 methods in a Template Method design pattern instance.  Base class deals
 with all locking issues in e.g. 'get' (the method a client calls),
 subclass can override _get and not worry about threading (it will be
 called by parent class's get with proper locks held and locks will be
 properly released c afterwards).

Ah, thank you - yes, that's the one I was thinking of. My brain was
telling me threading, which makes some sense, since I put the Queue
conceptually in the same bucket as the rest of the locking constructs in
the threading module.

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
---
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-06 Thread Nick Coghlan
Brian Quinlan wrote:
 - you need the cooperation of your subclasses i.e. they must call
   super().flush() in .flush() to get correct close behavior (and this
   represents a backwards-incompatible semantic change)

Are you sure about that? Going by the current _pyio semantics that
Antoine posted, it looks to me that it is already the case that
subclasses need to invoke the parent flush() call correctly to avoid
breaking the base class semantics (which really isn't an uncommon
problem when it comes to writing correct subclasses).

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
---
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-06 Thread Brian Quinlan

Nick Coghlan wrote:

Brian Quinlan wrote:

- you need the cooperation of your subclasses i.e. they must call
  super().flush() in .flush() to get correct close behavior (and this
  represents a backwards-incompatible semantic change)


Are you sure about that? Going by the current _pyio semantics that
Antoine posted, it looks to me that it is already the case that
subclasses need to invoke the parent flush() call correctly to avoid
breaking the base class semantics (which really isn't an uncommon
problem when it comes to writing correct subclasses).


As it is now, if you didn't call super().flush() in your flush override, 
then a buffer won't be flushed at the time that you expected.


With the proposed change, if you don't call super().flush() in your 
flush override, then the buffer will never get flushed and you will lose 
data when you close the file.


I'm not saying that it is a big deal, but it is a difference.

Cheers,
Brian
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-05 Thread Brian Quinlan

Hey Antoine,

Thanks for the clarification!

I see that the C implementation matches the Python implementation but I 
don't see how the semantics of either are useful in this case.


If a subclass implements flush then, as you say, it must also implement 
close and call flush itself before calling its superclass' close method. 
 But then _RawIOBase will pointlessly call the subclass' flush method a 
second time. This second call should raise (because the file is closed) 
and the exception will be caught and suppressed.


I don't see why this is helpful. Could you explain why 
_RawIOBase.close() calling self.flush() is useful?


Cheers,
Brian

Antoine Pitrou wrote:

Hi!

brian at sweetapp.com writes:

class _RawIOBase(_FileIO):


FileIO is a subclass of _RawIOBase, not the reverse:


issubclass(_io._RawIOBase, _io.FileIO)

False

issubclass(_io.FileIO, _io._RawIOBase)

True

I do understand your surprise, but the Python implementation of IOBase.close()
in _pyio.py does the same thing:

def close(self) - None:
Flush and close the IO object.

This method has no effect if the file is already closed.

if not self.__closed:
try:
self.flush()
except IOError:
pass  # If flush() fails, just give up
self.__closed = True

Note how it calls `self.flush()` and not `IOBase.flush(self)`.
When writing the C version of the I/O stack, we tried to keep the semantics the
same as in the Python version, although there are a couple of subtleties.

Your problem here is that it's IOBase.close() which calls your flush() method,
but FileIO.close() has already done its job before and the internal file
descriptor has been closed (hence `self.closed` is True). In this particular
case, I advocate overriding close() as well and call your flush() method
manually from there.

Thanks for your feedback!

Regards

Antoine.


___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/brian%40sweetapp.com


___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-05 Thread Antoine Pitrou
Brian Quinlan brian at sweetapp.com writes:
 
 I don't see why this is helpful. Could you explain why 
 _RawIOBase.close() calling self.flush() is useful?

I could not explain it for sure since I didn't write the Python version.
I suppose it's so that people who only override flush() automatically get the
flush-on-close behaviour.

cheers

Antoine.


___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-05 Thread Brian Quinlan

Antoine Pitrou wrote:

Brian Quinlan brian at sweetapp.com writes:
I don't see why this is helpful. Could you explain why 
_RawIOBase.close() calling self.flush() is useful?


I could not explain it for sure since I didn't write the Python version.
I suppose it's so that people who only override flush() automatically get the
flush-on-close behaviour.


But the way that the code is currently written, flush only gets called 
*after* the file has been closed (see my original example). It seems 
very unlikely that this is the behavior that the subclass would want/expect.


So any objections to me changing IOBase (and the C implementation) to:

def close(self):
Flush and close the IO object.

This method has no effect if the file is already closed.

if not self.__closed:
try:
-self.flush()
+IOBase.flush(self)
except IOError:
pass  # If flush() fails, just give up
self.__closed = True

Cheers,
Brian
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-05 Thread James Y Knight


On Apr 5, 2009, at 6:29 AM, Antoine Pitrou wrote:


Brian Quinlan brian at sweetapp.com writes:


I don't see why this is helpful. Could you explain why
_RawIOBase.close() calling self.flush() is useful?


I could not explain it for sure since I didn't write the Python  
version.
I suppose it's so that people who only override flush()  
automatically get the

flush-on-close behaviour.


It seems that a separate method _internal_close should've been  
defined to do the actual closing of the file, and the close() method  
should've been defined on the base class as self.flush();  
self._internal_close() and never overridden.


James
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-05 Thread Antoine Pitrou
James Y Knight foom at fuhm.net writes:
 
 It seems that a separate method _internal_close should've been  
 defined to do the actual closing of the file, and the close() method  
 should've been defined on the base class as self.flush();  
 self._internal_close() and never overridden.

I'm completely open to changes as long as there is a reasonable consensus around
them. Your proposal looks sane, although the fact that a semi-private method
(starting with an underscore) is designed to be overriden in some classes is a
bit annoying.

I'd also like to have some advice from Guido, since he was one of the driving
forces behind the specification and the original Python implementation.

Regards

Antoine.


___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-05 Thread Greg Ewing

Brian Quinlan wrote:


if not self.__closed:
try:
-self.flush()
+IOBase.flush(self)
except IOError:
pass  # If flush() fails, just give up
self.__closed = True


That doesn't seem like a good idea to me at all. If
someone overrides flush() but not close(), their
flush method won't get called, which would be surprising.

To get the desired behaviour, you need something like

  def close(self):
if not self.__closed:
  self.flush()
  self._close()
  self.__closed = True

and then tell people to override _close() rather than
close().

--
Greg
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-05 Thread Nick Coghlan
Antoine Pitrou wrote:
 James Y Knight foom at fuhm.net writes:
 It seems that a separate method _internal_close should've been  
 defined to do the actual closing of the file, and the close() method  
 should've been defined on the base class as self.flush();  
 self._internal_close() and never overridden.
 
 I'm completely open to changes as long as there is a reasonable consensus 
 around
 them. Your proposal looks sane, although the fact that a semi-private method
 (starting with an underscore) is designed to be overriden in some classes is a
 bit annoying.

Note that we already do that in a couple of places where it makes sense
- in those cases the underscore is there to tell *clients* of the class
don't call this directly, but it is still explicitly documented as
part of the subclassing API.

(the only example I can find at the moment is in asynchat, but I thought
there were a couple of more common ones than that - hopefully I'll think
of them later)

Cheers,
Nick.

-- 
Nick Coghlan   |   ncogh...@gmail.com   |   Brisbane, Australia
---
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-05 Thread Alex Martelli
On Sun, Apr 5, 2009 at 3:54 PM, Nick Coghlan ncogh...@gmail.com wrote:

 Antoine Pitrou wrote:
  James Y Knight foom at fuhm.net writes:
  It seems that a separate method _internal_close should've been
  defined to do the actual closing of the file, and the close() method
  should've been defined on the base class as self.flush();
  self._internal_close() and never overridden.
 
  I'm completely open to changes as long as there is a reasonable consensus
 around
  them. Your proposal looks sane, although the fact that a semi-private
 method
  (starting with an underscore) is designed to be overriden in some classes
 is a
  bit annoying.

 Note that we already do that in a couple of places where it makes sense
 - in those cases the underscore is there to tell *clients* of the class
 don't call this directly, but it is still explicitly documented as
 part of the subclassing API.

 (the only example I can find at the moment is in asynchat, but I thought
 there were a couple of more common ones than that - hopefully I'll think
 of them later)


Queue.Queue in 2.* (and queue.Queue in 3.*) is like that too -- the single
leading underscore meaning protected (I'm here for subclasses to override
me, only in C++ parlance) and a great way to denote hook methods in a
Template Method design pattern instance.  Base class deals with all locking
issues in e.g. 'get' (the method a client calls), subclass can override _get
and not worry about threading (it will be called by parent class's get with
proper locks held and locks will be properly released c afterwards).


Alex
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-05 Thread Greg Ewing

Antoine Pitrou wrote:

Your proposal looks sane, although the fact that a semi-private method
(starting with an underscore) is designed to be overriden in some classes is a
bit annoying.


The only other way I can see is to give up any attempt
in the base class to ensure that flushing occurs before
closing, and make that the responsibility of the derived
class.

--
Greg
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


[Python-Dev] Possible py3k io wierdness

2009-04-04 Thread brian
Hey,

I noticed that the call pattern of the C-implemented io libraries is as
follows (translating from C to Python):

class _FileIO(object):
  def flush(self):
if self.__IOBase_closed:
  raise ...

  def close(self):
self.flush()
self.__IOBase_closed = True

class _RawIOBase(_FileIO):
  def close(self):
# do close
_FileIO.close(self)

This means that, if a subclass overrides flush(), it will be called after
the file has been closed e.g.

 import io
 class MyIO(io.FileIO):
... def flush(self):
... print('closed:', self.closed)
...
 f = MyIO('test.out', 'wb')
 f.close()
closed: True

It seems to me that, during close, calls should only propagate up the
class hierarchy i.e.

class _FileIO(object):
  def flush(self):
if self.__IOBase_closed:
  raise ...

  def close(self):
_FileIO.flush(self)
self.__IOBase_closed = True

I volunteer to change this if there is agreement that this is the way to go.

Cheers,
Brian
___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com


Re: [Python-Dev] Possible py3k io wierdness

2009-04-04 Thread Antoine Pitrou
Hi!

brian at sweetapp.com writes:
 
 class _RawIOBase(_FileIO):

FileIO is a subclass of _RawIOBase, not the reverse:

 issubclass(_io._RawIOBase, _io.FileIO)
False
 issubclass(_io.FileIO, _io._RawIOBase)
True

I do understand your surprise, but the Python implementation of IOBase.close()
in _pyio.py does the same thing:

def close(self) - None:
Flush and close the IO object.

This method has no effect if the file is already closed.

if not self.__closed:
try:
self.flush()
except IOError:
pass  # If flush() fails, just give up
self.__closed = True

Note how it calls `self.flush()` and not `IOBase.flush(self)`.
When writing the C version of the I/O stack, we tried to keep the semantics the
same as in the Python version, although there are a couple of subtleties.

Your problem here is that it's IOBase.close() which calls your flush() method,
but FileIO.close() has already done its job before and the internal file
descriptor has been closed (hence `self.closed` is True). In this particular
case, I advocate overriding close() as well and call your flush() method
manually from there.

Thanks for your feedback!

Regards

Antoine.


___
Python-Dev mailing list
Python-Dev@python.org
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com