BaseIOStream.write(): support typed memoryviews#2996
BaseIOStream.write(): support typed memoryviews#2996bdarnell merged 1 commit intotornadoweb:masterfrom
Conversation
Making sure that ``len(data) == data.nbytes`` by casting memoryviews to bytes.
|
Sorry for the close and re-opening of the PR. We have fixed the issue in Dask/Distributed by always casting to bytes before communication: dask/distributed#4555 but I think this PR is still relevant for other downstream projects that might use typed |
bdarnell
left a comment
There was a problem hiding this comment.
Interesting, I hadn't noticed this aspect of memoryview before (I thought it was just a python-level exposure of the buffer protocol).
It looks like we could alternatively convert the input to a memoryview (unconditionally) and use nbytes instead of len. Is there any particular reason to prefer one form over the other? (I like replacing the union with a concrete type as early as possible, although len() is the natural idiom so choosing a type whose len() is problematic is error-prone).
I think both approaches make sense but if you are not planning to support non-contiguous buffers or fancy data compression, casting |
This PR implements support of
memoryviewswith an item size greater than 1 byte.Currently, Tornado uses
len(x)to retrieve the number of bytes ofx, which works fine whenxisbytesor amemoryviewwith an item size of 1. However, with an item size greater than 1 we cannot assume thatlen(x) == x.nbytes.