Fix animation errors #2898
Fix animation errors #2898
Conversation
| PyObject_CallFunction(write_method, (char *)"s#", pixBuffer, NUMBYTES); | ||
| #endif |
|
Before this gets merged some of the changes in #2838 should be reverted so that the smoke tests run again. |
| @@ -70,6 +70,7 @@ | |||
| Py_DECREF(ret); | |||
| fd = PyObject_AsFileDescriptor(file); | |||
| if (fd == -1) { | |||
| PyErr_Clear(); | |||
There was a problem hiding this comment.
I'm not sure this is the right solution. I think there are cases where you might want to return this error to the Python side. In cases where one doesn't, we are already calling PyErr_Clear in _png.cpp after calling mpl_PyFile_Dup. I think _backend_agg.cpp should also do that -- but I don't think it makes sense to do that here. The idea of this function is that a Python error should always be set when in returns NULL (like most Python/C API functions).
|
Thank for this! |
|
Fixes made. #2838 is not in v1.3.x, so I'm not reverting those changes here. Should I do a separate PR on master for that? |
|
Sorry, didn't notice where this was going. On 1.3.x the smoke tests are never run so it's not a 'problem'. 1.3.x will get merged into master so defiantly don't repeat the full PR. If you want to do a PR for reverting the |
|
Ok, #2900 takes care of the KnownFail. |
|
@mdboom This outside of my comfort zone to approve, if you are happy can you merge this? |
|
Ping? (Don't want to break with etiquette too badly, but I'd like to see this merged in :) |
|
My impression is that this PR has been vetted by @mdboom, and solves a problem up through Python 3.3, so maybe it is worth merging it now, into 1.3.x and from there into 1.4.x. If it is confirmed that there is a problem with 3.4, that can be treated separately. |
|
As noted in my comment on #1891, this PR needs one more change: move the /* Seek Python-side handle to the FILE* handle position */
ret = PyObject_CallMethod(file, "seek", MPL_OFF_T_PYFMT "i", position, 0);
if (ret == NULL) {
return -1;
}
Py_DECREF(ret);
}
return 0;That will ensure |
|
Fixed. |
|
@bytbox I don't know why Travis failed on Python 2, but it seems unrelated. (It looks familiar, and I think an explanation has been given, but I don't remember what it is.) Would you fix the indentation of the |
|
Oops - fixed the indentation. On Mon, 12 May 2014, Eric Firing wrote:
Scott Lawrence |
|
@tacaswell, will you take care of merging this into 1.4.x? And, if we are not going to release another 1.3.x, let's stop merging anything into it. |
|
I think we have one or two other PRs that are against 1.3.x, it is probably not worth rebasing them. I will merge this in tonight. |
|
I merged 1.3.x today so this should be taken care of. |
|
Just run into this bug - is this in 1.3 series or not in the end, seems the issue has not been closed... |
|
@mangecoeur Can you check using current master or the 1.4.0rc1 code? The fix for this was done after 1.3.1 was cut. |
|
@tacaswell can't change versions right now for various reasons, but I'm on 1.3.1 so that explains it... will wait for 1.4 release... |
Three changes: use 'y#' in python 3, ignore errors from AsFileDescriptor (to allow use of BytesIO and similarly brain-dead objects as the target), and don't require setting of the position of the file handle to succeed (so that we can use streams).
Fixes #1891