py/objarray: Add decode method to bytearray. #4771
Conversation
|
bytearray misses a lot of methods (should offer all methods of bytes). Proper resolution of that, if at all, would depend on resolving #4360 . But the usual comment would be that somehow we were able to live without them for years, so maybe MicroPython doesn't need them. |
|
Indeed, I'm fishing here: if there are others saying 'hey that decode() would be convenient' it might be ok for merging. Otherwise I can work around it. |
|
I'm +1 for this patch in its current shape (with configurable setting), as .decode() may be indeed useful. The point, there're more "missing" discoveries, and resolving them in such a way doesn't scale. |
py/objarray.c
Outdated
| @@ -548,7 +562,11 @@ const mp_obj_type_t mp_type_bytearray = { | |||
| .binary_op = array_binary_op, | |||
| .subscr = array_subscr, | |||
| .buffer_p = { .get_buffer = array_get_buffer }, | |||
| #if MICROPY_CPYTHON_COMPAT | |||
There was a problem hiding this comment.
The usual comment: please indent #if. (Yeah, I recently work on a project which doesn't follow this obvious rule, resulting in very ugly looking code, so am trained to recognize even minor cases of it.)
There was a problem hiding this comment.
Yes switching between projects and different styles makes me forgetting to do this correctly :[
There was a problem hiding this comment.
Instead of bytearray.decode() it's possible to use str(bytearray, 'utf8'). But I guess the point here is CPython compatibility.
If the method is to be added to bytearray, how about just putting .decode() in the existing array locals dict table? That would mean arrays now also have decode, but that can just be a "hidden feature", a MicroPython artefact of being efficient in code size. (And it'll work as expected because str(array, 'utf8') already works, even in CPython.)
py/objarray.c
Outdated
| @@ -195,6 +195,10 @@ STATIC mp_obj_t bytearray_make_new(const mp_obj_type_t *type_in, size_t n_args, | |||
| return array_construct(BYTEARRAY_TYPECODE, args[0]); | |||
| } | |||
| } | |||
|
|
|||
| #if MICROPY_CPYTHON_COMPAT | |||
| MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(bytearray_decode_obj, 1, 3, mp_obj_bytes_decode); | |||
There was a problem hiding this comment.
Why not just reuse the bytes_decode_obj wrapper?
Reuse the implementation for bytes since it works the same way irregardless of actual underlying types. This gets added for CPython compatibility of bytearray, but to keep the code simple and small, arrays now also has a working decode method which is non-standard but doesn't hurt.
That actually crossed my mind, much simpler than current implementation, just wasn't sure if the hidden decode() was ok. Together with your suggestion of reusing |
|
Thanks for updating, merged in fb54736 (with a small change to declare |
[resolves micropython#4771] DEBUG UART supported on ATMSAME5x
Reuse the implementation for bytes since it's exactly the same.
Also found this when working with msgpack where it gets used to get strings instead of bytes, which is too convenient not to use it (e.g.
msgpack.unpackb(msgpack.packb('a'))returnsb'a'without it). I don' think there is an easy way around here (like dynamically adding a method to a builtin type)?