classification
Title: Documentation/implementation out of sync for IO
Type: behavior Stage: resolved
Components: IO Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: martin.panter Nosy List: benjamin.peterson, martin.panter, pitrou, python-dev, serhiy.storchaka, stutzbach, viraptor
Priority: normal Keywords: patch

Created on 2014-11-12 15:06 by viraptor, last changed 2016-03-31 23:54 by martin.panter. This issue is now closed.

Files
File name Uploaded Description Edit
UnsupportedOperation.patch martin.panter, 2014-12-21 05:34 review
UnsupportedOperation.v2.patch martin.panter, 2016-03-18 02:01 review
UnsupportedOperation.v3.patch martin.panter, 2016-03-27 01:26 Change to OSError review
Messages (15)
msg231079 - (view) Author: Stanislaw Pitucha (viraptor) Date: 2014-11-12 15:06
The docstring on for fileno() method says:

"An IOError is raised if the IO object does not use a file descriptor."

In reality, UnsupportedOperation is raised instead:

```
: io.StringIO().fileno()
UnsupportedOperation: fileno
```
msg231080 - (view) Author: Stanislaw Pitucha (viraptor) Date: 2014-11-12 15:08
Just in case: yes, UnsupportedOperation is an IOError - but shouldn't docstring here be more specific?
msg231095 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2014-11-12 21:36
Similarly for the readable(), seekable() and writable() documentation
msg232989 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2014-12-21 05:34
Some of the docstrings already mention UnsupportedOperation. This patch updates the rest of the documentation. Also adds some tests to verify this on all the concrete classes I could think of. Some discoveries in the process:

* BufferedWriter.readable() and BufferedReader.writable() could return True depending on the underlying raw stream. Fixed to always return False.
* Removed a branch in a test case that assumed BufferedReader.close() did not call flush(), but never activated due to the above writable() bug
* seek(), tell() and truncate() do not raise UnsupportedOperation, despite seekable() == False, at least for a pipe. Adjusted doc strings.
msg261728 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-03-14 07:33
Nice patch. But it isn't applied cleanly on current tip (perhaps due to Argument Clinic). Added comments and questions on Rietveld.
msg261942 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-03-18 02:01
Thanks for looking at this Serhiy. Here is patch v2, merged with 3.6 branch; some doc string changes were redundant with upstream Arg Clinic changes.

However looking at this again, I think we should be cautious changing the documented exceptions for the base classes, since that is changing the API. E.g. perhaps it would be safer to leave the exception for fileno() as OSError (aka IOError). It it actually causing a problem? Or the API changes should be reserved for 3.6 only.
msg261953 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-03-18 10:02
I have doubts about changing OSError to UnsupportedOperation in the documentation of readable() and writable(). Some implementations can try to do an IO operation without checking readable() and writable() flags.

I have lesser doubt about using half-closed pipes in tests. May be it is safe, I don't know.
msg262503 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-03-27 01:26
Okay let’s document fileno(), read, write and seek operations in the base classes as raising OSError then. This effectively rejects the OP (Stanislaw’s) view that the exception should be more specific.

In patch v3, I changed everything over to say OSError is raised. I also added a background thread to drain the pipe writer. And I removed a test_no_fileno() method which was having an “existential crisis”. :)
msg262506 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-03-27 04:13
LGTM.
msg262690 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-31 09:49
New changeset dc9e5f09ac0c by Martin Panter in branch '3.5':
Issue #22854: Clarify documentation about UnsupportedOperation and add tests
https://hg.python.org/cpython/rev/dc9e5f09ac0c

New changeset c27e9dcad1a3 by Martin Panter in branch 'default':
Issue #22854: Merge UnsupportedOperation fixes from 3.5
https://hg.python.org/cpython/rev/c27e9dcad1a3
msg262691 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-31 10:00
New changeset 3d9d9ca75a31 by Martin Panter in branch '2.7':
Issue #22854: fileno() is always required in IOBase; remove test
https://hg.python.org/cpython/rev/3d9d9ca75a31
msg262692 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-31 10:55
New changeset fb10d1f5016e by Martin Panter in branch 'default':
Issue #22854: Skip pipe seekable() tests on Windows
https://hg.python.org/cpython/rev/fb10d1f5016e
msg262693 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-31 11:24
New changeset 66765a49465f by Martin Panter in branch '3.5':
Issue #22854: Skip pipe seek tests on Windows
https://hg.python.org/cpython/rev/66765a49465f

New changeset 3b7811b58a1f by Martin Panter in branch 'default':
Issue #22854: Merge Windows pipe skipping from 3.5
https://hg.python.org/cpython/rev/3b7811b58a1f
msg262694 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-03-31 11:32
I gave up on porting the fix to 2.7. Python 3 raises UnsupportedOperation (which inherits ValueError) in many cases due to Issue 9293, but Python 2 raises IOError (which does not inherit ValueError). Changing how BufferedWriter.read() etc work in Python 2 would break test_io_after_close(). Also, none of the doc strings in Python 2 need fixing.

Also according to the buildbots, Windows can seek in pipes. Or at least tell() doesn’t raise an exception. So I skipped the seek testing on Windows.
msg262708 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-03-31 21:24
New changeset b3c79e0ba477 by Martin Panter in branch '3.5':
Issue #22854: Fix logic for skipping test
https://hg.python.org/cpython/rev/b3c79e0ba477

New changeset de8412dc477e by Martin Panter in branch 'default':
Issue #22854: Merge test fix from 3.5
https://hg.python.org/cpython/rev/de8412dc477e
History
Date User Action Args
2016-03-31 23:54:06martin.pantersetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2016-03-31 21:24:12python-devsetmessages: + msg262708
2016-03-31 11:32:10martin.pantersetmessages: + msg262694
versions: - Python 2.7
2016-03-31 11:24:54python-devsetmessages: + msg262693
2016-03-31 10:55:12python-devsetmessages: + msg262692
2016-03-31 10:00:29python-devsetmessages: + msg262691
2016-03-31 09:49:58python-devsetnosy: + python-dev
messages: + msg262690
2016-03-27 04:13:58serhiy.storchakasetassignee: martin.panter
messages: + msg262506
stage: patch review -> commit review
2016-03-27 01:26:49martin.pantersetfiles: + UnsupportedOperation.v3.patch

messages: + msg262503
2016-03-18 10:02:30serhiy.storchakasetmessages: + msg261953
2016-03-18 02:01:09martin.pantersetfiles: + UnsupportedOperation.v2.patch

messages: + msg261942
2016-03-14 07:33:21serhiy.storchakasetnosy: + stutzbach, pitrou, serhiy.storchaka, benjamin.peterson

messages: + msg261728
versions: + Python 3.5, Python 3.6, - Python 3.2, Python 3.3, Python 3.4
2016-03-14 04:12:27martin.pantersetstage: patch review
2014-12-21 05:34:23martin.pantersetfiles: + UnsupportedOperation.patch
keywords: + patch
messages: + msg232989
2014-11-12 21:36:26martin.pantersetnosy: + martin.panter
messages: + msg231095
2014-11-12 15:08:34viraptorsetmessages: + msg231080
2014-11-12 15:06:27viraptorcreate