[REVIEW] tcp.write(): cast memoryview to "B" #4555
Conversation
Now that we depend on dask#4555, we don't need this hack.
madsbk
changed the title
Now that we depend on dask#4555, we don't need this hack.
madsbk
changed the title
|
Currently this gets handled as part of serialization distributed/distributed/protocol/core.py Line 85 in 7f8bb81 Would it be possible to address this in the MsgPack serialization work ( #4531 )? |
Sure, do you think it is better to do this when serializing? My idea was to do it for any message for the sake of robustness. |
|
I think it makes sense to do in serialization as we may want to capture information about the shape and type in the message we send. That way if it is important to have when reconstructing on the other side, we can shape and cast the array as expected. Otherwise the receiving side may not incorporate that information |
Yes, but I think we should let the specific serializers handle shape, type, etc. like the NumPy serializer does currently. The TCP just flattens the data to make sure that all the bytes are delivered to the peer correctly. |
madsbk
force-pushed
the
tcp_cast_to_1byte_itemsize
branch
from
482c642 to
a3c5794
Compare
|
Thanks Mads! 😄 |
* tcp.write(): cast memoryview to 1byte itemsize * dumps and loads now extract Serialize and Serialized * test_numpy: now use the new dumps scheme * Currently not handling bytes in dumps() * Removing extract_serialize() * test_comms: handle implicit list => tuple conversion by msgpack * Workaround a buffer length bug * Fixed some tuple/list mismatch in test_comms * test_compression_takes_advantage_of_itemsize(): xfail * to_serialize tests using bytearrays * Removing "B" cast of memoryviews hack Now that we depend on #4555, we don't need this hack. * TCP.write(): use nbytes() to determine if memoryview 0-size Co-authored-by: John Kirkham <jakirkham@gmail.com>
2 participants
This PR makes sure that memoryview given to Tornado's TCP stream have an item size of one. See tornadoweb/tornado#2996
Notice, since we are calling
stream._write_buffer.append()directly, we still need this PR even when tornadoweb/tornado#2996 get merged.black distributed/flake8 distributed