Change {FigureCanvasAgg,RendererAgg}.buffer_rgba to return a memoryview. #11735
Conversation
The `buffer_rgba` method now allows direct access to the renderer's underlying buffer (as a `(m, n, 4)`-shape memoryview) rather than copying the data to a new bytestring. This is consistent with the behavior on Py2, where a buffer object was returned. While this is technically a backwards-incompatible change, memoryviews are in fact quite compatible with bytes for most uses, and I'd argue that the bigger compatibility break was the change from no-copy in Py2 to copy in Py3 (after all that's the main point of the method...).
There was a problem hiding this comment.
Nice to remove some of the old backend_agg code here, especially as it is being replaced by built-in memoryviews.
I haven't looked at the impact of downstream users of tostring_argb etc., I assume that PIL happily consumes numpy.ndarray.tobytes and memoryviews, and that PIL will be the primary target for most direct users of these methods.
| @@ -15,7 +15,7 @@ | |||
| fig.canvas.draw() | |||
|
|
|||
| # grab the pixel buffer and dump it into a numpy array | |||
| X = np.array(fig.canvas.renderer._renderer) | |||
| X = np.array(fig.canvas.renderer.buffer_rgba()) | |||
|
|
||
| def tostring_argb(self): | ||
| return self._renderer.tostring_argb() | ||
| return np.asarray(self._renderer).take([3, 0, 1, 2], axis=2).tobytes() |
There was a problem hiding this comment.
Did you take a look at uses of this and tostring_rgb on things like StackOverflow to confirm that the type change would continue to work as expected in those use-cases?
There was a problem hiding this comment.
Here there is no change to the return type, it was a bytes, it's still a bytes.
There's a change in the return type of buffer_rgba() but that's sort of the whole point of the PR (and it goes back to the old Py2 behavior, and is also clearly better for performance).
|
Not sure about the squashing/merging policy. I'd say the changes are unrelated and can go in as separate PRs. However, I can only "squash and merge" or "create a merge commit". Naturally I'd use "rebase and merge", but that's not enabled for the repo. |
|
Why would you want to rebase and merge? It's the same as just merging, but more annoying. |
@QuLogic Not quite sure , if it would really be working the way I intended. But with a rebase the merge could be fast forward, so that the revision history stays more linear. |
The
buffer_rgbamethod now allows direct access to the renderer'sunderlying buffer (as a
(m, n, 4)-shape memoryview) rather thancopying the data to a new bytestring. This is consistent with the
behavior on Py2, where a buffer object was returned.
While this is technically a backwards-incompatible change, memoryviews
are in fact quite compatible with bytes for most uses, and I'd argue
that the bigger compatibility break was the change from no-copy in Py2
to copy in Py3 (after all that's the main point of the method...).
See also discussion in #11726.
Added second commit: reimplement tostring_rgba, tostring_rgb using numpy to access the buffer. Note that this was one of the suggestions of the original removal of PyCXX (#3646) :-)
PR Summary
PR Checklist