Re: [Python-Dev] Possible py3k io wierdness
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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