stephan-hof / pyrocksdb

Python bindings for RocksDB
BSD 3-Clause "New" or "Revised" License
150 stars 169 forks source link

Allow bytearray as input #35

Open Downchuck opened 8 years ago

Downchuck commented 8 years ago

Currently the bytes_to_slice does not support bytearray; bytearray is just a standard/compatible pointer.

batch.put(key, bytearray("data"))
Downchuck commented 8 years ago

Possibly related: https://github.com/cython/cython/blob/master/Cython/Includes/cpython/bytes.pxd#L77

Seems to suggest that arbitrary binary data would be converted to Unicode. EDIT: Doesn't seem to be an issue. Still, it seems that a direct cast to char* would work fine.

Downchuck commented 8 years ago

Not sure how safe it is, but this lets me pass in bytearray:

+++ b/rocksdb/_rocksdb.pyx
 cdef Slice bytes_to_slice(ob) except *:
-    return Slice(PyBytes_AsString(ob), PyBytes_Size(ob))
+    return Slice(<char*>ob, len(ob))
Downchuck commented 8 years ago

@stephan-hof What are your thoughts on this one? You could do an if(isinstance(ob, bytearray)) to support it.

stephan-hof commented 8 years ago

Hi @Downchuck I think its a good idea to support byte-array (and possibly memoryview, buffer, ...). I have also seen that you opened/commented on other issues, so I guess you are currently working with the stuff. Unfortunately I'm currently super busy with other things, so I don't have much time to dive into the issues right now. I'll have a look at them end of next week.

Downchuck commented 8 years ago

I tried to support memoryview in another cython project -- I did not succeed.

bytearray was easy, as it was just a raw char*. It seems like memoryview and buffer both need their own work, futher, I was trying to pass a memoryviewslice from another cython build (via runcython).

Anyway, that single-line item seems to handle python bytes() and bytearray().