[Jython-checkins] jython: Partly treat ValueError/IOError confusion in _io.FileIO
jeff.allen
jython-checkins at python.org
Sun Dec 16 01:20:05 CET 2012
http://hg.python.org/jython/rev/4bfdc027aad2
changeset: 6900:4bfdc027aad2
user: Jeff Allen <ja...py at farowl.co.uk>
date: Thu Dec 13 16:31:05 2012 +0000
summary:
Partly treat ValueError/IOError confusion in _io.FileIO
Reworked the tests for closed, readable and writable in PyFileIO to be more like CPython.
Corresponding changes to some tests. More fun with mode strings. test_io.py now scores
fail/error/skip = 12/15/99, and test_fileio 0/0/1 (but with some concessions to Jython
non-conformance still in place).
files:
Lib/_jyio.py | 5 +-
Lib/test/test_fileio.py | 5 +-
src/org/python/modules/_io/OpenMode.java | 23 -
src/org/python/modules/_io/PyFileIO.java | 154 ++++++++--
src/org/python/modules/_io/PyIOBase.java | 43 ++-
src/org/python/modules/_io/_io.java | 2 +-
6 files changed, 160 insertions(+), 72 deletions(-)
diff --git a/Lib/_jyio.py b/Lib/_jyio.py
--- a/Lib/_jyio.py
+++ b/Lib/_jyio.py
@@ -193,8 +193,9 @@
def close(self):
if self.raw is not None and not self.closed:
try:
- # may raise BlockingIOError or BrokenPipeError etc
- self.flush()
+ # Jython difference: call super.close() which manages "closed to client" state,
+ # and calls flush(), which may raise BlockingIOError or BrokenPipeError etc.
+ super(_BufferedIOBase, self).close()
finally:
self.raw.close()
diff --git a/Lib/test/test_fileio.py b/Lib/test/test_fileio.py
--- a/Lib/test/test_fileio.py
+++ b/Lib/test/test_fileio.py
@@ -427,10 +427,7 @@
self.assertEqual(w.warnings, [])
self.assertRaises(TypeError, _FileIO, [])
self.assertEqual(w.warnings, [])
- if is_jython:
- self.assertRaises(IOError, _FileIO, "/some/invalid/name", "rt")
- else:
- self.assertRaises(ValueError, _FileIO, "/some/invalid/name", "rt")
+ self.assertRaises(ValueError, _FileIO, "/some/invalid/name", "rt")
self.assertEqual(w.warnings, [])
def test_main():
diff --git a/src/org/python/modules/_io/OpenMode.java b/src/org/python/modules/_io/OpenMode.java
--- a/src/org/python/modules/_io/OpenMode.java
+++ b/src/org/python/modules/_io/OpenMode.java
@@ -40,12 +40,6 @@
/** Set true when any invalid symbol or combination is discovered */
public boolean invalid;
- /** Set true when stream must be <code>readable = reading | updating</code> */
- public boolean readable;
-
- /** Set true when stream must be <code>writable = writing | updating | appending</code> */
- public boolean writable;
-
/**
* Error message describing the way in which the mode is invalid, or null if no problem has been
* found. This field may be set by the constructor (in the case of duplicate or unrecognised
@@ -139,8 +133,6 @@
// Implications
reading |= universal;
- readable = reading | updating;
- writable = writing | updating | appending;
// Standard tests
if (!invalid) {
@@ -208,21 +200,6 @@
}
/**
- * The mode string that a raw file should claim to have, when initialised with the present mode.
- * Note that this is not the same as the open mode because it omits the text-based attributes,
- * even if set, and always asserts it is binary.
- *
- * @return "rb", "rb+", or "wb".
- */
- public String raw() {
- if (readable) {
- return writable ? "rb+" : "rb";
- } else {
- return "wb";
- }
- }
-
- /**
* The mode string we need when constructing a <code>FileIO</code> initialised with the present
* mode. Note that this is not the same as the full open mode because it omits the text-based
* attributes, and not the same as {@link #raw()}.
diff --git a/src/org/python/modules/_io/PyFileIO.java b/src/org/python/modules/_io/PyFileIO.java
--- a/src/org/python/modules/_io/PyFileIO.java
+++ b/src/org/python/modules/_io/PyFileIO.java
@@ -9,6 +9,7 @@
import org.python.core.BuiltinDocs;
import org.python.core.Py;
import org.python.core.PyBuffer;
+import org.python.core.PyException;
import org.python.core.PyJavaType;
import org.python.core.PyLong;
import org.python.core.PyNewWrapper;
@@ -37,7 +38,24 @@
@ExposedGet(doc = BuiltinDocs.file_name_doc)
protected PyObject name;
- private Boolean seekable;
+ /*
+ * Implementation note: CPython fileio does not use the base-class, possibly overridden,
+ * readable(), writable() and seekable(). Instead it sets local variables for readable and
+ * writable using the open mode, and returns these as readable() and writable(), while using
+ * them internally. The local variable seekable (and seekable()) is worked out from a one-time
+ * trial seek.
+ */
+ /** Set true when stream must be <code>readable = reading | updating</code> */
+ private boolean readable;
+
+ /** Set true when stream must be <code>writable = writing | updating | appending</code> */
+ private boolean writable;
+
+ /** Set true when we have made the seekable test */
+ private boolean seekableKnown;
+
+ /** Set true when stream is seekable */
+ private boolean seekable;
/** Whether to close the underlying stream on closing this object. */
@ExposedGet
@@ -45,8 +63,11 @@
/** The mode as given to the constructor */
private OpenMode openMode;
- @ExposedGet(doc = BuiltinDocs.file_mode_doc) // and as a PyString
- public PyString mode() { return new PyString(openMode.raw()); }
+
+ /** The mode as a PyString based on readable and writable */
+ @ExposedGet(doc = BuiltinDocs.file_mode_doc)
+ public final PyString mode;
+
private static final PyString defaultMode = new PyString("r");
/**
@@ -82,6 +103,16 @@
this.closefd = closefd;
this.name = file;
this.openMode = mode;
+
+ readable = mode.reading | mode.updating;
+ writable = mode.writing | mode.updating | mode.appending;
+
+ // The mode string of a raw file always asserts it is binary: "rb", "rb+", or "wb".
+ if (readable) {
+ this.mode = new PyString(writable ? "rb+" : "rb");
+ } else {
+ this.mode = new PyString("wb");
+ }
}
/**
@@ -124,9 +155,9 @@
*/
Channel channel = ((RawIOBase)fd).getChannel();
if (channel instanceof FileChannel) {
- if (channel.isOpen()){
- FileChannel fc = (FileChannel)channel;
- return new FileIO(fc, mode.forFileIO());
+ if (channel.isOpen()) {
+ FileChannel fc = (FileChannel)channel;
+ return new FileIO(fc, mode.forFileIO());
} else {
// File not open (we have to check as FileIO doesn't)
throw Py.OSError(Errno.EBADF);
@@ -146,6 +177,7 @@
@ExposedNew
static PyObject FileIO___new__(PyNewWrapper new_, boolean init, PyType subtype,
PyObject[] args, String[] keywords) {
+
ArgParser ap = new ArgParser("FileIO", args, keywords, openArgs, 1);
PyObject file = ap.getPyObject(0);
PyObject m = ap.getPyObject(1, defaultMode);
@@ -168,7 +200,6 @@
}
-
/*
* ===========================================================================================
* Exposed methods in the order they appear in CPython's fileio.c method table
@@ -185,9 +216,11 @@
@ExposedMethod(doc = readinto_doc)
final PyLong FileIO_readinto(PyObject buf) {
- // Check we can do this
- _checkClosed();
- _checkReadable();
+
+ if (!readable) { // ... (or closed)
+ throw tailoredValueError("read");
+ }
+
// Perform the operation through a buffer view on the object
PyBuffer pybuf = writablePyBuffer(buf);
try {
@@ -198,6 +231,7 @@
count = ioDelegate.readinto(byteBuffer);
}
return new PyLong(count);
+
} finally {
// Must unlock the PyBuffer view from client's object
pybuf.release();
@@ -211,7 +245,11 @@
@ExposedMethod(doc = write_doc)
final PyLong FileIO_write(PyObject obj) {
- _checkWritable();
+
+ if (!writable) { // ... (or closed)
+ throw tailoredValueError("writ");
+ }
+
// Get or synthesise a buffer API on the object to be written
PyBuffer pybuf = readablePyBuffer(obj);
try {
@@ -223,6 +261,7 @@
count = ioDelegate.write(byteBuffer);
}
return new PyLong(count);
+
} finally {
// Even if that went badly, we should release the lock on the client buffer
pybuf.release();
@@ -246,6 +285,9 @@
/** Common to FileIO_truncate(null) and truncate(). */
private final long _truncate() {
+ if (!writable) { // ... (or closed)
+ throw tailoredValueError("writ");
+ }
synchronized (ioDelegate) {
return ioDelegate.truncate(ioDelegate.tell());
}
@@ -253,6 +295,9 @@
/** Common to FileIO_truncate(size) and truncate(size). */
private final long _truncate(long size) {
+ if (!writable) { // ... (or closed)
+ throw tailoredValueError("writ");
+ }
synchronized (ioDelegate) {
return ioDelegate.truncate(size);
}
@@ -275,21 +320,42 @@
if (closefd) {
ioDelegate.close();
}
+ // This saves us doing two tests for each action (when the file is open)
+ readable = false;
+ writable = false;
}
@Override
- public boolean readable() {
+ public boolean readable() throws PyException {
return FileIO_readable();
}
- @ExposedMethod(doc = "True if file was opened in a read mode.")
+ @ExposedMethod(doc = readable_doc)
final boolean FileIO_readable() {
- return ioDelegate.readable();
+ if (__closed) {
+ throw closedValueError();
+ }
+ return readable;
+ }
+
+ @Override
+ public boolean writable() throws PyException {
+ return FileIO_writable();
+ }
+
+ @ExposedMethod(doc = writable_doc)
+ final boolean FileIO_writable() {
+ if (__closed) {
+ throw closedValueError();
+ }
+ return writable;
}
@ExposedMethod(defaults = {"0"}, doc = BuiltinDocs.file_seek_doc)
final synchronized PyObject FileIO_seek(long pos, int how) {
- _checkClosed();
+ if (__closed) {
+ throw closedValueError();
+ }
return Py.java2py(ioDelegate.seek(pos, how));
}
@@ -300,15 +366,21 @@
@ExposedMethod(doc = "True if file supports random-access.")
final boolean FileIO_seekable() {
- if (seekable == null) {
+ if (__closed) {
+ throw closedValueError();
+ }
+ if (!seekableKnown) {
seekable = ioDelegate.seek(0, 0) >= 0;
+ seekableKnown = true;
}
return seekable;
}
@ExposedMethod(doc = BuiltinDocs.file_tell_doc)
final synchronized long FileIO_tell() {
- _checkClosed();
+ if (__closed) {
+ throw closedValueError();
+ }
return ioDelegate.tell();
}
@@ -322,28 +394,24 @@
return FileIO_isatty();
}
- @ExposedMethod(doc = BuiltinDocs.file_isatty_doc)
+ @ExposedMethod(doc = isatty_doc)
final boolean FileIO_isatty() {
+ if (__closed) {
+ throw closedValueError();
+ }
return ioDelegate.isatty();
}
@Override
- public boolean writable() {
- return FileIO_writable();
- }
-
- @ExposedMethod(doc = "True if file was opened in a write mode.")
- final boolean FileIO_writable() {
- return ioDelegate.writable();
- }
-
- @Override
public PyObject fileno() {
return FileIO_fileno();
}
@ExposedMethod(doc = BuiltinDocs.file_fileno_doc)
final PyObject FileIO_fileno() {
+ if (__closed) {
+ throw closedValueError();
+ }
return PyJavaType.wrapJavaObject(ioDelegate.fileno());
}
@@ -367,9 +435,9 @@
final String FileIO_toString() {
if (name instanceof PyUnicode) {
String escapedName = PyString.encode_UnicodeEscape(name.toString(), false);
- return String.format("<_io.FileIO name='%s', mode='%s'>", escapedName, mode());
+ return String.format("<_io.FileIO name='%s', mode='%s'>", escapedName, mode);
}
- return String.format("<_io.FileIO name='%s', mode='%s'>", name, mode());
+ return String.format("<_io.FileIO name='%s', mode='%s'>", name, mode);
}
@Override
@@ -377,4 +445,30 @@
return FileIO_toString();
}
+ /**
+ * Convenience method providing the exception when an method requires the file to be open, and
+ * it isn't.
+ *
+ * @return ValueError to throw
+ */
+ private PyException closedValueError() {
+ return Py.ValueError("I/O operation on closed file");
+ }
+
+ /**
+ * Convenience method providing the exception when an method requires the file to be open,
+ * readable or writable, and it isn't. If the file is closed, return the message for that,
+ * otherwise, one about reading or writing.
+ *
+ * @param action type of operation not valid ("read" or "writ" in practice).
+ * @return ValueError to throw
+ */
+ private PyException tailoredValueError(String action) {
+ if (action == null || __closed) {
+ return closedValueError();
+ } else {
+ return Py.ValueError("File not open for " + action + "ing");
+ }
+ }
+
}
diff --git a/src/org/python/modules/_io/PyIOBase.java b/src/org/python/modules/_io/PyIOBase.java
--- a/src/org/python/modules/_io/PyIOBase.java
+++ b/src/org/python/modules/_io/PyIOBase.java
@@ -223,13 +223,14 @@
* Is the stream capable of positioning the read/write pointer?
*
* @return <code>True</code> if may be positioned
+ * @throws PyException(ValueError) if the object is closed to client operations
*/
- public boolean seekable() {
+ public boolean seekable() throws PyException {
return _IOBase_seekable();
}
@ExposedMethod(doc = seekable_doc)
- final boolean _IOBase_seekable() {
+ final boolean _IOBase_seekable() throws PyException {
return false;
}
@@ -238,6 +239,8 @@
* positioned.
*
* @param msg optional custom message
+ * @throws PyException(ValueError) if the object is closed to client operations
+ * @throws PyException(IOError) if the stream <b>is not</b> capable of being positioned.
*/
public void _checkSeekable(String msg) {
_IOBase__checkSeekable(msg);
@@ -246,6 +249,9 @@
/**
* Raise an error if the pointer of underlying IO stream <b>is not</b> capable of being
* positioned.
+ *
+ * @throws PyException(ValueError) if the object is closed to client operations
+ * @throws PyException(IOError) if the stream <b>is not</b> capable of being positioned.
*/
public final void _checkSeekable() {
_checkSeekable(null);
@@ -262,13 +268,14 @@
* Is the stream readable?
*
* @return <code>true</code> if readable
+ * @throws PyException(ValueError) if the object is closed to client operations
*/
- public boolean readable() {
+ public boolean readable() throws PyException {
return _IOBase_readable();
}
@ExposedMethod(doc = readable_doc)
- final boolean _IOBase_readable() {
+ final boolean _IOBase_readable() throws PyException {
return false;
}
@@ -276,6 +283,8 @@
* Raise an error if the underlying IO stream <b>is not</b> readable.
*
* @param msg optional custom message
+ * @throws PyException(ValueError) if the object is closed to client operations
+ * @throws PyException(IOError) if the stream <b>is not</b> readable.
*/
public void _checkReadable(String msg) {
_IOBase__checkReadable(msg);
@@ -283,6 +292,9 @@
/**
* Raise an error if the underlying IO stream <b>is not</b> readable.
+ *
+ * @throws PyException(ValueError) if the object is closed to client operations
+ * @throws PyException(IOError) if the stream <b>is not</b> readable.
*/
public final void _checkReadable() {
_checkReadable(null);
@@ -299,13 +311,14 @@
* Is the stream writable?
*
* @return <code>true</code> if writable
+ * @throws PyException(ValueError) if the object is closed to client operations
*/
- public boolean writable() {
+ public boolean writable() throws PyException {
return _IOBase_writable();
}
@ExposedMethod(doc = writable_doc)
- final boolean _IOBase_writable() {
+ final boolean _IOBase_writable() throws PyException {
return false;
}
@@ -313,20 +326,25 @@
* Raise an error if the underlying IO stream <b>is not</b> writable.
*
* @param msg optional custom message
+ * @throws PyException(ValueError) if the object is closed to client operations
+ * @throws PyException(IOError) if the stream <b>is not</b> writable.
*/
- public void _checkWritable(String msg) {
+ public void _checkWritable(String msg) throws PyException {
_IOBase__checkWritable(msg);
}
/**
* Raise an error if the underlying IO stream <b>is not</b> writable.
+ *
+ * @throws PyException(ValueError) if the object is closed to client operations
+ * @throws PyException(IOError) if the stream <b>is not</b> writable.
*/
- public final void _checkWritable() {
+ public final void _checkWritable() throws PyException {
_checkWritable(null);
}
@ExposedMethod(defaults = "null")
- final void _IOBase__checkWritable(String msg) {
+ final void _IOBase__checkWritable(String msg) throws PyException {
if (!invoke("writable").__nonzero__()) {
throw tailoredIOError(msg, "writ");
}
@@ -346,17 +364,18 @@
* {@link #_checkSeekable}, etc..
*
* @param msg optional custom message
+ * @throws PyException(ValueError) if the object is closed to client operations
*/
- public void _checkClosed(String msg) {
+ public void _checkClosed(String msg) throws PyException {
_IOBase__checkClosed(msg);
}
- public final void _checkClosed() {
+ public final void _checkClosed() throws PyException {
_checkClosed(null);
}
@ExposedMethod(defaults = "null")
- final void _IOBase__checkClosed(String msg) {
+ final void _IOBase__checkClosed(String msg) throws PyException {
if (closed()) {
throw Py.ValueError(msg != null ? msg : "I/O operation on closed file");
}
diff --git a/src/org/python/modules/_io/_io.java b/src/org/python/modules/_io/_io.java
--- a/src/org/python/modules/_io/_io.java
+++ b/src/org/python/modules/_io/_io.java
@@ -167,7 +167,7 @@
if (mode.updating) {
bufferType = io.__getattr__("BufferedRandom");
- } else if (mode.writable) { // = writing || appending
+ } else if (mode.writing || mode.appending) {
bufferType = io.__getattr__("BufferedWriter");
} else { // = reading
bufferType = io.__getattr__("BufferedReader");
--
Repository URL: http://hg.python.org/jython
More information about the Jython-checkins
mailing list