Fix reading/writing from urllib.request objects #5910
Conversation
| @@ -517,6 +517,13 @@ def test_minimized_rasterized(): | |||
| assert False | |||
|
|
|||
|
|
|||
| @cleanup | |||
There was a problem hiding this comment.
doesn't this need a @attr('network') decorator too?
There was a problem hiding this comment.
Indeed. I forgot we had that.
| @@ -206,7 +206,17 @@ static PyObject *Py_write_png(PyObject *self, PyObject *args, PyObject *kwds) | |||
| goto exit; | |||
| } | |||
| buff.cursor = 0; | |||
| } else if ((fp = mpl_PyFile_Dup(py_file, (char *)"wb", &offset))) { | |||
| } else { | |||
| #if PY3K | |||
There was a problem hiding this comment.
Just remembered seeing an article recently imploring that we stop writing if-statements like this that would possibly break in py4.x. The logic really should be checking whether or not it is py2.x, as py2.x should be considered the exception to the rule.
There was a problem hiding this comment.
Nope -- this won't break. Here's the definition of it in mpl_utils.h:
#if PY_MAJOR_VERSION >= 3
#define PY3K 1
#define Py_TPFLAGS_HAVE_NEWBUFFER 0
#else
#define PY3K 0
#endifI take PY3K to mean "next-gen Python" as it was called before Python 3.0 was even released.
|
This all looks good to me. The appveyor failures are the usual ones. Thanks for the quick turnaround! I'll also backport to v1.5.x branch. |
Fix reading/writing from urllib.request objects
|
Ok, actually, the backport isn't totally clean. I wonder why? |
|
Looks like we never backported this code portion: Was that intentional? |
|
Does git blame tell us where it is coming from? |
|
looks like b3e9443 |
|
So that traces it to #5389 which is mainly a performance improvement and thus not backported. Not sure how to best handle that |
|
Looking over it, I wonder if that was the commit that broke things? So, On Mon, Jan 25, 2016 at 3:37 PM, Jens Hedegaard Nielsen <
|
|
According to the original issue mail the bug is on 1.4.0 and up so I guess that |
|
hmmm, that's right. How about I take a crack at the backport and come up with a PR for it so that we can double-check that it makes sense? |
Yes -- it's an optimization for the nbagg backend. It doesn't make sense to backport that unless we backport the related changes to nbagg (which I wouldn't recommend). |
Fix reading/writing from urllib.request objects This backport needed to exclude nbagg optimizations.
|
#5927 is my attempt at backporting to v1.5.x |
Merge pull request #5910 from mdboom/image-read-from-url
Fix reading/writing from urllib.request objects This backport needed to exclude nbagg optimizations.
Faster image generation in WebAgg/NbAgg backends Conflicts: src/_png.cpp Due to having previously backported matplotlib#5910 ( 3675841 ) as ( 99ad89d )
Addresses matplotlib-devel thread "Possible bug in matplotlib versions 1.4 and higher with basemap arcgisimage imagery"