Git Product home page Git Product logo

Comments (15)

fbergmann avatar fbergmann commented on June 8, 2024 1

running the same package just with pyodide works just fine. Additionally, with a local installation, replacing that unicode character in the pyscript files, the package also works as planned. Also an older version of pyscript works fine. In fact the same package on pyscript.com works as well:

https://frank_bergmann.pyscriptapps.com/copasi-test/latest/

from pyscript.

WebReflection avatar WebReflection commented on June 8, 2024 1

Pinging @fpliger then as we need to be careful with Unicode chars in comments, apparently, but I will also investigate why that breaks .. afaik we do test pymedia and we have no issue there

from pyscript.

fbergmann avatar fbergmann commented on June 8, 2024 1

thank you very much, I can confirm that the patched version linked above works fine.

from pyscript.

fbergmann avatar fbergmann commented on June 8, 2024

I think it comes from here:

pyscript/pyscript.core/src/stdlib/pyweb/media.py

Line 77 in d7084f7

group identifier if they belong to the same physical device — for

from pyscript.

WebReflection avatar WebReflection commented on June 8, 2024

the issue happens only with that package so it's definitively not an issue with our own code because it's tested and it works ... it is possible though that our packages to virtual FileSystem fails with some edge case but it's hard from this issue to understand what is it that is causing troubles.

from pyscript.

WebReflection avatar WebReflection commented on June 8, 2024

please also keep in mind packages are given directly to pyodide and its import mechanism so I think you'll have the same isseu in raw pyodide and, if that's the case, this issue should be filed in there.

from pyscript.

fpliger avatar fpliger commented on June 8, 2024

Mmmm

That's a weird one, definitely worth investing

from pyscript.

WebReflection avatar WebReflection commented on June 8, 2024

OK, I did investigate and I can't reproduce any issue there ... this is my current test page, it uses both the legacy way to write python code and the modern script one, just to exclude possible shenanigans caused by browsers parsing the content of the <py-script> custom tag and the result is the same ... I see those lines printed, including the incriminated char:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8" />
    <meta name="viewport" content="width=device-width,initial-scale=1" />
    <link
        rel="stylesheet"
        href="https://pyscript.net/releases/2024.1.3/core.css"
    />
    <script
        type="module"
        src="https://pyscript.net/releases/2024.1.3/core.js"
    ></script>
  </head>
  <body>
    <py-script>
        from pyweb import media
        print("from pyodide.ffi import to_js\nfrom pyscript import window\n\n\nclass Device:\n    \"\"\"Device represents a media input or output device, such as a microphone,\n    camera, or headset.\n    \"\"\"\n\n    def __init__(self, device):\n        self._js = device\n\n    @property\n    def id(self):\n        return self._js.deviceId\n\n    @property\n    def group(self):\n        return self._js.groupId\n\n    @property\n    def kind(self):\n        return self._js.kind\n\n    @property\n    def label(self):\n        return self._js.label\n\n    def __getitem__(self, key):\n        return getattr(self, key)\n\n    @classmethod\n    async def load(cls, audio=False, video=True):\n        \"\"\"Load the device stream.\"\"\"\n        options = window.Object.new()\n        options.audio = audio\n        if isinstance(video, bool):\n            options.video = video\n        else:\n            # TODO: Think this can be simplified but need to check it on the pyodide side\n\n            # TODO: this is pyodide specific. shouldn't be!\n            options.video = window.Object.new()\n            for k in video:\n                setattr(\n                    options.video,\n                    k,\n                    to_js(video[k], dict_converter=window.Object.fromEntries),\n                )\n\n        stream = await window.navigator.mediaDevices.getUserMedia(options)\n        return stream\n\n    async def get_stream(self):\n        key = self.kind.replace(\"input\", \"\").replace(\"output\", \"\")\n        options = {key: {\"deviceId\": {\"exact\": self.id}}}\n\n        return await self.load(**options)\n\n\nasync def list_devices() -> list[dict]:\n    \"\"\"\n    Return the list of the currently available media input and output devices,\n    such as microphones, cameras, headsets, and so forth.\n\n    Output:\n\n        list(dict) - list of dictionaries representing the available media devices.\n            Each dictionary has the following keys:\n            * deviceId: a string that is an identifier for the represented device\n                that is persisted across sessions. It is un-guessable by other\n                applications and unique to the origin of the calling application.\n                It is reset when the user clears cookies (for Private Browsing, a\n                different identifier is used that is not persisted across sessions).\n\n            * groupId: a string that is a group identifier. Two devices have the same\n                group identifier if they belong to the same physical device — for\n                example a monitor with both a built-in camera and a microphone.\n\n            * kind: an enumerated value that is either \"videoinput\", \"audioinput\"\n                or \"audiooutput\".\n\n            * label: a string describing this device (for example \"External USB\n                Webcam\").\n\n    Note: the returned list will omit any devices that are blocked by the document\n    Permission Policy: microphone, camera, speaker-selection (for output devices),\n    and so on. Access to particular non-default devices is also gated by the\n    Permissions API, and the list will omit devices for which the user has not\n    granted explicit permission.\n    \"\"\"\n    # https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/enumerateDevices\n    return [\n        Device(obj) for obj in await window.navigator.mediaDevices.enumerateDevices()\n    ]\n")
    </py-script>
    <script type="py">
        from pyweb import media
        print("from pyodide.ffi import to_js\nfrom pyscript import window\n\n\nclass Device:\n    \"\"\"Device represents a media input or output device, such as a microphone,\n    camera, or headset.\n    \"\"\"\n\n    def __init__(self, device):\n        self._js = device\n\n    @property\n    def id(self):\n        return self._js.deviceId\n\n    @property\n    def group(self):\n        return self._js.groupId\n\n    @property\n    def kind(self):\n        return self._js.kind\n\n    @property\n    def label(self):\n        return self._js.label\n\n    def __getitem__(self, key):\n        return getattr(self, key)\n\n    @classmethod\n    async def load(cls, audio=False, video=True):\n        \"\"\"Load the device stream.\"\"\"\n        options = window.Object.new()\n        options.audio = audio\n        if isinstance(video, bool):\n            options.video = video\n        else:\n            # TODO: Think this can be simplified but need to check it on the pyodide side\n\n            # TODO: this is pyodide specific. shouldn't be!\n            options.video = window.Object.new()\n            for k in video:\n                setattr(\n                    options.video,\n                    k,\n                    to_js(video[k], dict_converter=window.Object.fromEntries),\n                )\n\n        stream = await window.navigator.mediaDevices.getUserMedia(options)\n        return stream\n\n    async def get_stream(self):\n        key = self.kind.replace(\"input\", \"\").replace(\"output\", \"\")\n        options = {key: {\"deviceId\": {\"exact\": self.id}}}\n\n        return await self.load(**options)\n\n\nasync def list_devices() -> list[dict]:\n    \"\"\"\n    Return the list of the currently available media input and output devices,\n    such as microphones, cameras, headsets, and so forth.\n\n    Output:\n\n        list(dict) - list of dictionaries representing the available media devices.\n            Each dictionary has the following keys:\n            * deviceId: a string that is an identifier for the represented device\n                that is persisted across sessions. It is un-guessable by other\n                applications and unique to the origin of the calling application.\n                It is reset when the user clears cookies (for Private Browsing, a\n                different identifier is used that is not persisted across sessions).\n\n            * groupId: a string that is a group identifier. Two devices have the same\n                group identifier if they belong to the same physical device — for\n                example a monitor with both a built-in camera and a microphone.\n\n            * kind: an enumerated value that is either \"videoinput\", \"audioinput\"\n                or \"audiooutput\".\n\n            * label: a string describing this device (for example \"External USB\n                Webcam\").\n\n    Note: the returned list will omit any devices that are blocked by the document\n    Permission Policy: microphone, camera, speaker-selection (for output devices),\n    and so on. Access to particular non-default devices is also gated by the\n    Permissions API, and the list will omit devices for which the user has not\n    granted explicit permission.\n    \"\"\"\n    # https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/enumerateDevices\n    return [\n        Device(obj) for obj in await window.navigator.mediaDevices.enumerateDevices()\n    ]\n")
    </script>
  </body>
</html>

I did also manually changed that char with \u2014 directly in the comment and the result is the same: nothing breaks.

I am afraid being incapable to reproduce this issue makes it very hard to fix it ... what is happening in your package exactly and why would our otherwise-working code fail? 🤔

from pyscript.

WebReflection avatar WebReflection commented on June 8, 2024

P.S. also note that output as attribute doesn't exist anymore ... it's eventually target and it's an ID or a CSS selector.

edit same goes for py-env that doesn't exist anymore

from pyscript.

WebReflection avatar WebReflection commented on June 8, 2024

I tracked it out to the boilerscript code generated, that includes _path.write_text statements without an encoding specified, and inside that string, there are those unicode characters.

about this ... I've tried to enforce latin-1 without success ... meaning: no errors.

I am struggling to understand in which circumstance you have issues because that is a valid JSON string and JSON is pretty universal. I understand that char in particular goes beyond ASCII table but I don't understand why you have such issue.

If enforcing utf-8 encoding instead would solve without affecting too much bootstrap performance, I might try to fix it but I still would love to understand the culprit of this issue as no browser I could try with no version could reproduce this issue.

from pyscript.

fbergmann avatar fbergmann commented on June 8, 2024

I'd be happy to change the wheel as well, but i fail to understand where the problem lies. I essentially built the package using pyodide build, tested it in pyodide (and jupyterlite), and then wanted to try it with pyscript, where it worked with the old version but not the new one.

The package itself is a scientific library, exposed via SWIG. I don't understand what would change the environment default encoding by just using the py-config line with the package statement. If it helps, here is the __init__.py script, though i'm not sure that would have executed at that point.

https://gist.github.com/fbergmann/48219beb795a42d2c17294c03d803fac

if there is any further information i could provide, please let me know.

from pyscript.

WebReflection avatar WebReflection commented on June 8, 2024

I am not sure I have resources to dig into a 21K lines of code but these lines got me thinking ... https://gist.github.com/fbergmann/48219beb795a42d2c17294c03d803fac#file-__init__-py-L100-L103

I have no idea what's going on in there but if comments are not dropped out of the equation that might be the issue ... I don't think with older PyScript we were sanitizing more than what JSON can produce, but again ... if you think the issue is in the write_text not specifying any encoding, I am down to try a variant but without a minimal reproducible test, I can't just land that variant in production to let you test your module works ... I hope this makes sense.

from pyscript.

WebReflection avatar WebReflection commented on June 8, 2024

on a second thought, you @fbergmann could do that!

you clone this repo, run npm i and eventually npm run build ... then you run npm run server and you have all you need.

You can then add utf-8 encoding in this line https://github.com/pyscript/pyscript/blob/main/pyscript.core/src/stdlib.js#L24 as extra embedded argument, run npm run build again, and test your stuff works.

I understand I am asking a lot from you but please try to realize without a minimal reproducible code, and to date tons of modules have been used without this issue, it's extremely hard to validate the workaround or fix.

I am here to help you forward with any local test, so please let me know if you need anything else.

P.S. any ./test/ file that is html likely gives you a template to bootstrap from there and see if latest built artifact satisfies your use-case.

from pyscript.

fbergmann avatar fbergmann commented on June 8, 2024

I can confirm that running with the patch, everything works as it should. While without it I still see the error.

diff --git a/pyscript.core/src/stdlib.js b/pyscript.core/src/stdlib.js
index 4818772..dc5d9da 100644
--- a/pyscript.core/src/stdlib.js
+++ b/pyscript.core/src/stdlib.js
@@ -21,7 +21,7 @@ const write = (base, literal) => {
         python.push(`_path = _Path("${base}/${key}")`);
         if (typeof value === "string") {
             const code = JSON.stringify(value);
-            python.push(`_path.write_text(${code})`);
+            python.push(`_path.write_text(${code}, encoding="utf-8")`);
         } else {
             // @see https://github.com/pyscript/pyscript/pull/1813#issuecomment-1781502909
             python.push(`if not _os.path.exists("${base}/${key}"):`);

Not sure how to measure performance. I ran the tests using make test-integration with the patch the runtime was:

============ 152 passed, 40 skipped, 6 xfailed in 203.21s (0:03:23) ============

without

============ 152 passed, 40 skipped, 6 xfailed in 211.59s (0:03:31) ============

but I'm not sure whether that difference is representative. You let me know what else you'd need from me.

from pyscript.

WebReflection avatar WebReflection commented on June 8, 2024

First of all, thank you a lot for helping out, really appreciated!

We have merged the patched version on main and published to npm (our "canary" distribution).

You can use these CDN links to try latest and re-confirm we're good now:

from pyscript.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.