Distributive-Network / PythonMonkey

A Mozilla SpiderMonkey JavaScript engine embedded into the Python VM, using the Python engine to provide the JS host environment.
https://pythonmonkey.io
Other
854 stars 40 forks source link

Can't Convert Python Bytes to a PythonMonkey Type #385

Closed wiwichips closed 3 months ago

wiwichips commented 4 months ago

Issue type

Bug

How did you install PythonMonkey?

Source

OS platform and distribution

Ubuntu 22.04.4 LTS x86_64

Python version (python --version)

3.10

PythonMonkey version (pip show pythonmonkey)

Version: 0.6.0

Bug Description

No response

Standalone code to reproduce the issue

>>> import pythonmonkey as pm
>>> pm.globalThis.console.log(bytes("hello world", "ascii"))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
BufferError: Object is not writable.

Relevant log output or backtrace

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
BufferError: Object is not writable.

Additional info if applicable

- Also occurs on newest build from source
- Python `bytes` is a pretty common type and it would be pleasant if it worked. For instance, when you read a file it comes in as bytes in Python
- Wes said this is a bug in the private dcp distributive slack in #Python-Monkey channel: https://dstrbtv.slack.com/archives/C03RNFRL4NQ/p1721165437856109
- What does it mean to have an immutable byte buffer in JS - do we need a new type which implements readonly uint8array methods?
- This is (probably?) lower priority issue than the corrupted HTTP headers bug 
- I experienced this bug while playing around with different values from Python to PythonMonkey

What branch of PythonMonkey were you developing on? (If applicable)

No response

philippedistributive commented 4 months ago

This is not supported.

See https://github.com/Distributive-Network/PythonMonkey/blob/main/src/BufferType.cc#L99 and https://github.com/Distributive-Network/PythonMonkey/blob/main/tests/python/test_buffer_typed_array.py#L192

so we don't want to treat an immutable object as mutable...yet

zollqir commented 4 months ago

This could be supported by implementing a JS type that works like a JS TypedArray but is immutable

philippedistributive commented 4 months ago

@wiwichips as I asked in slack here: https://dstrbtv.slack.com/archives/C03RNFRL4NQ/p1721168301362849 did this occur in some specific context/task or just while playing around? Is this useful in bifrost 2?

wiwichips commented 4 months ago

@philippedistributive check out the additional info section on the ticket

In short yes, this would be nice to support in bifrost2, however it's not required - we can use copy bytes into bytebuffers instead


Would it be a terrible idea to implement a ImmutableUint8Array ?

wiwichips commented 4 months ago

conversation continued here: https://dstrbtv.slack.com/archives/C03RNFRL4NQ/p1721247135158099


maybe doesn't make sense to proxy python bytes into js as some new immutable or frozen type -

philippedistributive commented 4 months ago

and here https://dstrbtv.slack.com/archives/C03RNFRL4NQ/p1721306139024959

philippedistributive commented 4 months ago

Philippe Laporte So we cannot use bytearray instead of bytes correct?

Will So yeah we can use bytearrays - the only problem is that we'll often be casting bytes to bytearrays which is a copy under the hood These buffers could be quite large, and it would be nice to avoid copies by passing the reference to pythonmonkey instead