Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

_png.read_png crashes on Python 3 with urllib.request object #1650

Merged
merged 5 commits into from

3 participants

@cgohlke

The following code crashes on Python 3.x but not on Python 2.x (verified on Windows with mpl 1.2.0):

url = 'http://www.libpng.org/pub/png/img_png/pngnow.png'
try:
    import urllib2
    data = urllib2.urlopen(url)
except Exception:
    import urllib.request
    data = urllib.request.urlopen(url)

from matplotlib import pyplot

image = pyplot.imread(data)  # crash on py3.x
pyplot.imshow(image)
pyplot.show()

The crash is at _png.cpp line 328

Maybe using numpy's npy_PyFile_* functions could help. A lot of thought has gone into those functions.

@mdboom mdboom Fixes #1650 where using a file-like object on Python 3 fails. Use npy…
…_PyFile_* compatibility methods instead of rolling it ourselves.
889951e
@mdboom
Owner

Thanks for the tip about npy_PyFile. I wasn't aware of those. Using those does seem to fix things (and simplifies our code). Once you've confirmed this works for you, I'll go ahead and merge. The merge back to master is going to be tricky, as the _png extension has been largely rewritten over there.

@cgohlke

Thanks, that was fast. It does work for me. Verified with matplotlib-1.2.0.win-amd64-py3.3.exe.
Does this change the numpy version required by matplotlib?

@mdboom
Owner

Ugh... I didn't realise -- it appears that npy_PyFile_CloseFile is only in Numpy 1.7. That's not going to fly. The other functions go back to 1.5.0, which is better, but still higher than the 1.4.0 which is our current minimum requirement.

I guess we copy these over to our own tree? I'd hate to do that...

@cgohlke

This needs more testing. In IPython 0.13.1 notebook on Python 3.3 now I get:

In [5]: pyplot.plot()
Out[5]:
[]
---------------------------------------------------------------------------
UnsupportedOperation                      Traceback (most recent call last)
X:\Python33\lib\site-packages\IPython\zmq\pylab\backend_inline.py in show(close)
    100     try:
    101         for figure_manager in Gcf.get_all_fig_managers():
--> 102             send_figure(figure_manager.canvas.figure)
    103     finally:
    104         show._to_draw = []

X:\Python33\lib\site-packages\IPython\zmq\pylab\backend_inline.py in send_figure(fig)
    209     """
    210     fmt = InlineBackend.instance().figure_format
--> 211     data = print_figure(fig, fmt)
    212     # print_figure will return None if there's nothing to draw:
    213     if data is None:

X:\Python33\lib\site-packages\IPython\core\pylabtools.py in print_figure(fig, fmt)
    102     try:
    103         bytes_io = BytesIO()
--> 104         fig.canvas.print_figure(bytes_io, format=fmt, bbox_inches='tight')
    105         data = bytes_io.getvalue()
    106     finally:

X:\Python33\lib\site-packages\matplotlib\backend_bases.py in print_figure(self, filename, dpi, facecolor, edgecolor, orientation, format, **kwargs)
   2050                     orientation=orientation,
   2051                     dryrun=True,
-> 2052                     **kwargs)
   2053                 renderer = self.figure._cachedRenderer
   2054                 bbox_inches = self.figure.get_tightbbox(renderer)

X:\Python33\lib\site-packages\matplotlib\backends\backend_agg.py in print_png(self, filename_or_obj, *args, **kwargs)
    501             _png.write_png(renderer._renderer.buffer_rgba(),
    502                            renderer.width, renderer.height,
--> 503                            filename_or_obj, self.figure.dpi)
    504         finally:
    505             if close:

UnsupportedOperation: fileno
@mdboom
Owner

Ok -- seems to be reproduceable on Python 3.2 as well, with IPython 0.13. IPython git master fails to display inline plots on Python 3.x for an apparently unrelated reason.

In any case, I have a fix for this. When a "fake" file object (such as a BytesIO here) is passed in, PyFile_Dup will fail and set an exception. This exception needs to be cleared because we have a fallback method for handling these fake files.

@cgohlke

I just noticed that files are not opened in binary mode for reading and writing. I think for Windows the file open modes must be "rb" and "wb" (maybe I am missing something).

@mdboom
Owner

Good catch. I've updated the PR to open the files in binary mode.

@mdboom mdboom merged commit 293d42b into from
@pelson
Collaborator

@mdboom - have you done a merge back to master recently? I haven't - it's probably a good idea to get on top of that hurdle as soon as we can if you haven't either.

@mdboom
Owner

I did this a few times yesterday, and it seems we're caught up at the moment. I agree with you -- it's usually best to merge as soon as possible to avoid large, complex conflicts.

@mdboom mdboom deleted the branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 10, 2013
  1. @mdboom

    Fixes #1650 where using a file-like object on Python 3 fails. Use npy…

    mdboom authored
    …_PyFile_* compatibility methods instead of rolling it ourselves.
Commits on Jan 11, 2013
  1. @mdboom
  2. @mdboom
  3. @mdboom
  4. @mdboom

    Open files in binary mode.

    mdboom authored
This page is out of date. Refresh to see the latest.
Showing with 213 additions and 77 deletions.
  1. +24 −25 src/_backend_agg.cpp
  2. +54 −52 src/_png.cpp
  3. +135 −0 src/file_compat.h
View
49 src/_backend_agg.cpp
@@ -39,6 +39,7 @@
#include "numpy/arrayobject.h"
#include "agg_py_transforms.h"
+#include "file_compat.h"
#ifndef M_PI
#define M_PI 3.14159265358979323846
@@ -2028,44 +2029,42 @@ RendererAgg::write_rgba(const Py::Tuple& args)
FILE *fp = NULL;
Py::Object py_fileobj = Py::Object(args[0]);
-
- #if PY3K
- int fd = PyObject_AsFileDescriptor(py_fileobj.ptr());
- PyErr_Clear();
- #endif
+ PyObject* py_file = NULL;
+ bool close_file = false;
if (py_fileobj.isString())
{
- std::string fileName = Py::String(py_fileobj);
- const char *file_name = fileName.c_str();
- if ((fp = fopen(file_name, "wb")) == NULL)
- throw Py::RuntimeError(
- Printf("Could not open file %s", file_name).str());
- if (fwrite(pixBuffer, 1, NUMBYTES, fp) != NUMBYTES)
- {
- fclose(fp);
- throw Py::RuntimeError(
- Printf("Error writing to file %s", file_name).str());
+ if ((py_file = npy_PyFile_OpenFile(py_fileobj.ptr(), (char *)"wb")) == NULL) {
+ throw Py::Exception();
}
+ close_file = true;
}
- #if PY3K
- else if (fd != -1)
+ else
{
- if (write(fd, pixBuffer, NUMBYTES) != (ssize_t)NUMBYTES)
- {
- throw Py::RuntimeError("Error writing to file");
- }
+ py_file = py_fileobj.ptr();
}
- #else
- else if (PyFile_CheckExact(py_fileobj.ptr()))
+
+ if ((fp = npy_PyFile_Dup(py_file, (char *)"wb")))
{
- fp = PyFile_AsFile(py_fileobj.ptr());
if (fwrite(pixBuffer, 1, NUMBYTES, fp) != NUMBYTES)
{
+ npy_PyFile_DupClose(py_file, fp);
+
+ if (close_file) {
+ npy_PyFile_CloseFile(py_file);
+ Py_DECREF(py_file);
+ }
+
throw Py::RuntimeError("Error writing to file");
}
+
+ npy_PyFile_DupClose(py_file, fp);
+
+ if (close_file) {
+ npy_PyFile_CloseFile(py_file);
+ Py_DECREF(py_file);
+ }
}
- #endif
else
{
PyObject* write_method = PyObject_GetAttrString(py_fileobj.ptr(),
View
106 src/_png.cpp
@@ -27,6 +27,7 @@
#include "CXX/Extensions.hxx"
#include "numpy/arrayobject.h"
#include "mplutils.h"
+#include "file_compat.h"
// As reported in [3082058] build _png.so on aix
#ifdef _AIX
@@ -104,6 +105,7 @@ Py::Object _png_module::write_png(const Py::Tuple& args)
FILE *fp = NULL;
bool close_file = false;
+ bool close_dup_file = false;
Py::Object buffer_obj = Py::Object(args[0]);
PyObject* buffer = buffer_obj.ptr();
if (!PyObject_CheckReadBuffer(buffer))
@@ -128,41 +130,34 @@ Py::Object _png_module::write_png(const Py::Tuple& args)
}
Py::Object py_fileobj = Py::Object(args[3]);
-#if PY3K
- int fd = PyObject_AsFileDescriptor(py_fileobj.ptr());
- PyErr_Clear();
-#endif
+ PyObject* py_file = NULL;
if (py_fileobj.isString())
{
- std::string fileName = Py::String(py_fileobj);
- const char *file_name = fileName.c_str();
- if ((fp = fopen(file_name, "wb")) == NULL)
- {
- throw Py::RuntimeError(
- Printf("Could not open file %s", file_name).str());
+ if ((py_file = npy_PyFile_OpenFile(py_fileobj.ptr(), (char *)"wb")) == NULL) {
+ throw Py::Exception();
}
close_file = true;
}
-#if PY3K
- else if (fd != -1)
+ else
{
- fp = fdopen(fd, "w");
+ py_file = py_fileobj.ptr();
}
-#else
- else if (PyFile_CheckExact(py_fileobj.ptr()))
+
+ if ((fp = npy_PyFile_Dup(py_file, (char *)"wb")))
{
- fp = PyFile_AsFile(py_fileobj.ptr());
+ close_dup_file = true;
}
-#endif
else
{
+ PyErr_Clear();
PyObject* write_method = PyObject_GetAttrString(
- py_fileobj.ptr(), "write");
+ py_file, "write");
if (!(write_method && PyCallable_Check(write_method)))
{
Py_XDECREF(write_method);
throw Py::TypeError(
- "Object does not appear to be a 8-bit string path or a Python file-like object");
+ "Object does not appear to be a 8-bit string path or "
+ "a Python file-like object");
}
Py_XDECREF(write_method);
}
@@ -205,7 +200,7 @@ Py::Object _png_module::write_png(const Py::Tuple& args)
}
else
{
- png_set_write_fn(png_ptr, (void*)py_fileobj.ptr(),
+ png_set_write_fn(png_ptr, (void*)py_file,
&write_png_data, &flush_png_data);
}
png_set_IHDR(png_ptr, info_ptr,
@@ -241,9 +236,16 @@ Py::Object _png_module::write_png(const Py::Tuple& args)
png_destroy_write_struct(&png_ptr, &info_ptr);
}
delete [] row_pointers;
- if (fp && close_file)
+
+ if (close_dup_file)
+ {
+ npy_PyFile_DupClose(py_file, fp);
+ }
+
+ if (close_file)
{
- fclose(fp);
+ npy_PyFile_CloseFile(py_file);
+ Py_DECREF(py_file);
}
/* Changed calls to png_destroy_write_struct to follow
http://www.libpng.org/pub/png/libpng-manual.txt.
@@ -254,15 +256,15 @@ Py::Object _png_module::write_png(const Py::Tuple& args)
png_destroy_write_struct(&png_ptr, &info_ptr);
delete [] row_pointers;
-#if PY3K
- if (fp)
+ if (close_dup_file)
{
- fflush(fp);
+ npy_PyFile_DupClose(py_file, fp);
}
-#endif
- if (fp && close_file)
+
+ if (close_file)
{
- fclose(fp);
+ npy_PyFile_CloseFile(py_file);
+ Py_DECREF(py_file);
}
if (PyErr_Occurred()) {
@@ -306,40 +308,33 @@ _png_module::_read_png(const Py::Object& py_fileobj, const bool float_result,
png_byte header[8]; // 8 is the maximum size that can be checked
FILE* fp = NULL;
bool close_file = false;
-
-#if PY3K
- int fd = PyObject_AsFileDescriptor(py_fileobj.ptr());
- PyErr_Clear();
-#endif
+ bool close_dup_file = false;
+ PyObject *py_file = NULL;
if (py_fileobj.isString())
{
- std::string fileName = Py::String(py_fileobj);
- const char *file_name = fileName.c_str();
- if ((fp = fopen(file_name, "rb")) == NULL)
- {
- throw Py::RuntimeError(
- Printf("Could not open file %s for reading", file_name).str());
+ if ((py_file = npy_PyFile_OpenFile(py_fileobj.ptr(), (char *)"rb")) == NULL) {
+ throw Py::Exception();
}
close_file = true;
+ } else {
+ py_file = py_fileobj.ptr();
}
-#if PY3K
- else if (fd != -1) {
- fp = fdopen(fd, "r");
- }
-#else
- else if (PyFile_CheckExact(py_fileobj.ptr()))
+
+ if ((fp = npy_PyFile_Dup(py_file, "rb")))
{
- fp = PyFile_AsFile(py_fileobj.ptr());
+ close_dup_file = true;
}
-#endif
else
{
- PyObject* read_method = PyObject_GetAttrString(py_fileobj.ptr(), "read");
+ PyErr_Clear();
+ PyObject* read_method = PyObject_GetAttrString(py_file, "read");
if (!(read_method && PyCallable_Check(read_method)))
{
Py_XDECREF(read_method);
- throw Py::TypeError("Object does not appear to be a 8-bit string path or a Python file-like object");
+ throw Py::TypeError(
+ "Object does not appear to be a 8-bit string path or a Python "
+ "file-like object");
}
Py_XDECREF(read_method);
}
@@ -354,7 +349,7 @@ _png_module::_read_png(const Py::Object& py_fileobj, const bool float_result,
}
else
{
- _read_png_data(py_fileobj.ptr(), header, 8);
+ _read_png_data(py_file, header, 8);
}
if (png_sig_cmp(header, 0, 8))
{
@@ -390,7 +385,7 @@ _png_module::_read_png(const Py::Object& py_fileobj, const bool float_result,
}
else
{
- png_set_read_fn(png_ptr, (void*)py_fileobj.ptr(), &read_png_data);
+ png_set_read_fn(png_ptr, (void*)py_file, &read_png_data);
}
png_set_sig_bytes(png_ptr, 8);
png_read_info(png_ptr, info_ptr);
@@ -572,10 +567,17 @@ _png_module::_read_png(const Py::Object& py_fileobj, const bool float_result,
#else
png_destroy_read_struct(&png_ptr, &info_ptr, png_infopp_NULL);
#endif
+ if (close_dup_file)
+ {
+ npy_PyFile_DupClose(py_file, fp);
+ }
+
if (close_file)
{
- fclose(fp);
+ npy_PyFile_CloseFile(py_file);
+ Py_DECREF(py_file);
}
+
for (row = 0; row < height; row++)
{
delete [] row_pointers[row];
View
135 src/file_compat.h
@@ -0,0 +1,135 @@
+#ifndef __FILE_COMPAT_H__
+#define __FILE_COMPAT_H__
+
+#include "numpy/npy_3kcompat.h"
+
+#if NPY_API_VERSION < 0x4 /* corresponds to Numpy 1.5 */
+/*
+ * PyFile_* compatibility
+ */
+#if defined(NPY_PY3K)
+
+/*
+ * Get a FILE* handle to the file represented by the Python object
+ */
+static NPY_INLINE FILE*
+npy_PyFile_Dup(PyObject *file, char *mode)
+{
+ int fd, fd2;
+ PyObject *ret, *os;
+ Py_ssize_t pos;
+ FILE *handle;
+ /* Flush first to ensure things end up in the file in the correct order */
+ ret = PyObject_CallMethod(file, "flush", "");
+ if (ret == NULL) {
+ return NULL;
+ }
+ Py_DECREF(ret);
+ fd = PyObject_AsFileDescriptor(file);
+ if (fd == -1) {
+ return NULL;
+ }
+ os = PyImport_ImportModule("os");
+ if (os == NULL) {
+ return NULL;
+ }
+ ret = PyObject_CallMethod(os, "dup", "i", fd);
+ Py_DECREF(os);
+ if (ret == NULL) {
+ return NULL;
+ }
+ fd2 = PyNumber_AsSsize_t(ret, NULL);
+ Py_DECREF(ret);
+#ifdef _WIN32
+ handle = _fdopen(fd2, mode);
+#else
+ handle = fdopen(fd2, mode);
+#endif
+ if (handle == NULL) {
+ PyErr_SetString(PyExc_IOError,
+ "Getting a FILE* from a Python file object failed");
+ }
+ ret = PyObject_CallMethod(file, "tell", "");
+ if (ret == NULL) {
+ fclose(handle);
+ return NULL;
+ }
+ pos = PyNumber_AsSsize_t(ret, PyExc_OverflowError);
+ Py_DECREF(ret);
+ if (PyErr_Occurred()) {
+ fclose(handle);
+ return NULL;
+ }
+ npy_fseek(handle, pos, SEEK_SET);
+ return handle;
+}
+
+/*
+ * Close the dup-ed file handle, and seek the Python one to the current position
+ */
+static NPY_INLINE int
+npy_PyFile_DupClose(PyObject *file, FILE* handle)
+{
+ PyObject *ret;
+ Py_ssize_t position;
+ position = npy_ftell(handle);
+ fclose(handle);
+
+ ret = PyObject_CallMethod(file, "seek", NPY_SSIZE_T_PYFMT "i", position, 0);
+ if (ret == NULL) {
+ return -1;
+ }
+ Py_DECREF(ret);
+ return 0;
+}
+
+static NPY_INLINE int
+npy_PyFile_Check(PyObject *file)
+{
+ int fd;
+ fd = PyObject_AsFileDescriptor(file);
+ if (fd == -1) {
+ PyErr_Clear();
+ return 0;
+ }
+ return 1;
+}
+
+#else
+
+#define npy_PyFile_Dup(file, mode) PyFile_AsFile(file)
+#define npy_PyFile_DupClose(file, handle) (0)
+
+#endif
+
+static NPY_INLINE PyObject*
+npy_PyFile_OpenFile(PyObject *filename, const char *mode)
+{
+ PyObject *open;
+ open = PyDict_GetItemString(PyEval_GetBuiltins(), "open");
+ if (open == NULL) {
+ return NULL;
+ }
+ return PyObject_CallFunction(open, "Os", filename, mode);
+}
+
+#endif /* NPY_API_VERSION < 0x4 */
+
+#if NPY_API_VERSION < 0x7 /* corresponds to Numpy 1.7 */
+
+static NPY_INLINE int
+npy_PyFile_CloseFile(PyObject *file)
+{
+ PyObject *ret;
+
+ ret = PyObject_CallMethod(file, "close", NULL);
+ if (ret == NULL) {
+ return -1;
+ }
+ Py_DECREF(ret);
+ return 0;
+}
+
+#endif /* NPY_API_VERSION < 0x7 */
+
+#endif /* ifndef __FILE_COMPAT_H__ */
Something went wrong with that request. Please try again.