From 158376f7104c6381aa648c53a2e9ce83aa2f5f24 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Mon, 26 Dec 2022 10:20:05 -0800 Subject: [PATCH] Improve mypy typing for pyodide package (#3385) The way we were handling the "routing" in `_core.py` (where in browser things are imported from `_pyodide_core` and otherwise things from `_pyodide._core_docs`) made mypy type a bunch of things as `Any`. This switches to doing the imports from `_pyodide_core` with reflection that mypy cannot understand so it will correctly type all of the methods with their types from `_pyodide._core_docs`. This created some new type errors, so I also made various fixes to make things typecheck again. --- docs/project/changelog.md | 3 + src/py/_pyodide/_core_docs.py | 140 +++++++++++++++++++++++++----- src/py/js.pyi | 24 ++--- src/py/pyodide/_core.py | 93 +++++++++++--------- src/py/pyodide/ffi/wrappers.py | 49 ++++++----- src/py/pyodide/http.py | 25 +++--- src/tests/test_pyodide.py | 4 +- src/tests/test_typeconversions.py | 2 +- 8 files changed, 230 insertions(+), 110 deletions(-) diff --git a/docs/project/changelog.md b/docs/project/changelog.md index cea5ec859..14544c949 100644 --- a/docs/project/changelog.md +++ b/docs/project/changelog.md @@ -114,6 +114,9 @@ substitutions: `js` module. {pr}`3298` +- {{ Enhancement }} mypy understands the types of more things now. + {pr}`3385` + - {{ Fix }} Fixed bug in `split` argument of {any}`repr_shorten`. Added {any}`shorten` function. {pr}`3178` diff --git a/src/py/_pyodide/_core_docs.py b/src/py/_pyodide/_core_docs.py index 5e43ad2d8..d3ebb9469 100644 --- a/src/py/_pyodide/_core_docs.py +++ b/src/py/_pyodide/_core_docs.py @@ -8,11 +8,12 @@ from collections.abc import ( KeysView, Mapping, MutableMapping, + Sequence, ValuesView, ) from functools import reduce from types import TracebackType -from typing import IO, Any, Awaitable +from typing import IO, Any, Awaitable, overload # All docstrings for public `core` APIs should be extracted from here. We use # the utilities in `docstring.py` and `docstring.c` to format them @@ -24,23 +25,6 @@ _save_name = __name__ __name__ = "pyodide" -# From jsproxy.c -class JsException(Exception): - """ - A wrapper around a JavaScript Error to allow it to be thrown in Python. - See :ref:`type-translations-errors`. - """ - - @property - def js_error(self) -> "JsProxy": - """The original JavaScript error""" - return JsProxy(_instantiate_token) - - -class ConversionError(Exception): - """An error thrown when conversion between JavaScript and Python fails.""" - - _js_flags: dict[str, int] = {} @@ -857,10 +841,71 @@ class JsAsyncGenerator(JsIterable): raise NotImplementedError +class JsCallable(JsProxy): + _js_type_flags = ["IS_CALLABLE"] + + def __call__(self): + pass + + +class JsOnceCallable(JsCallable): + def destroy(self): + pass + + +class JsRawException(JsProxy): + @property + def name(self) -> str: + return "" + + @property + def message(self) -> str: + return "" + + @property + def stack(self) -> str: + return "" + + +class JsException(Exception): + """ + A wrapper around a JavaScript Error to allow it to be thrown in Python. + See :ref:`type-translations-errors`. + """ + + @property + def js_error(self) -> JsRawException: + """The original JavaScript error""" + return JsRawException(_instantiate_token) + + +class ConversionError(Exception): + """An error thrown when conversion between JavaScript and Python fails.""" + + +class JsDomElement(JsProxy): + @property + def tagName(self) -> str: + return "" + + @property + def children(self) -> Sequence["JsDomElement"]: + return [] + + def appendChild(self, child: "JsDomElement") -> None: + pass + + def addEventListener(self, event: str, listener: Callable[[Any], None]) -> None: + pass + + def removeEventListener(self, event: str, listener: Callable[[Any], None]) -> None: + pass + + # from pyproxy.c -def create_once_callable(obj: Callable[..., Any], /) -> JsProxy: +def create_once_callable(obj: Callable[..., Any], /) -> JsOnceCallable: """Wrap a Python callable in a JavaScript function that can be called once. After being called the proxy will decrement the reference count @@ -909,6 +954,41 @@ def create_proxy( # from python2js +@overload +def to_js( + obj: list[Any] | tuple[Any], + /, + *, + depth: int = -1, + pyproxies: JsProxy | None = None, + create_pyproxies: bool = True, + dict_converter: Callable[[Iterable[JsArray]], JsProxy] | None = None, + default_converter: Callable[ + [Any, Callable[[Any], JsProxy], Callable[[Any, JsProxy], None]], JsProxy + ] + | None = None, +) -> JsArray: + ... + + +@overload +def to_js( + obj: dict[Any, Any], + /, + *, + depth: int = -1, + pyproxies: JsProxy | None, + create_pyproxies: bool, + dict_converter: None, + default_converter: Callable[ + [Any, Callable[[Any], JsProxy], Callable[[Any, JsProxy], None]], JsProxy + ] + | None = None, +) -> JsMap: + ... + + +@overload def to_js( obj: Any, /, @@ -916,12 +996,28 @@ def to_js( depth: int = -1, pyproxies: JsProxy | None = None, create_pyproxies: bool = True, - dict_converter: Callable[[Iterable[JsProxy]], JsProxy] | None = None, + dict_converter: Callable[[Iterable[JsArray]], JsProxy] | None = None, default_converter: Callable[ [Any, Callable[[Any], JsProxy], Callable[[Any, JsProxy], None]], JsProxy ] | None = None, -) -> JsProxy: +) -> Any: + ... + + +def to_js( + obj: Any, + /, + *, + depth: int = -1, + pyproxies: JsProxy | None = None, + create_pyproxies: bool = True, + dict_converter: Callable[[Iterable[JsArray]], JsProxy] | None = None, + default_converter: Callable[ + [Any, Callable[[Any], JsProxy], Callable[[Any, JsProxy], None]], JsProxy + ] + | None = None, +) -> Any: """Convert the object to JavaScript. This is similar to :any:`PyProxy.toJs`, but for use from Python. If the @@ -1029,7 +1125,7 @@ def to_js( return obj -def destroy_proxies(pyproxies: JsProxy, /) -> None: +def destroy_proxies(pyproxies: JsArray, /) -> None: """Destroy all PyProxies in a JavaScript array. pyproxies must be a JsProxy of type PyProxy[]. Intended for use with the diff --git a/src/py/js.pyi b/src/py/js.pyi index 23a7e4f39..63f8b1278 100644 --- a/src/py/js.pyi +++ b/src/py/js.pyi @@ -2,7 +2,14 @@ from asyncio import Future from typing import Any, Callable, Iterable from _pyodide._core_docs import _JsProxyMetaClass -from pyodide.ffi import JsArray, JsBuffer, JsFetchResponse, JsProxy, JsTypedArray +from pyodide.ffi import ( + JsArray, + JsBuffer, + JsDomElement, + JsFetchResponse, + JsProxy, + JsTypedArray, +) from pyodide.webloop import PyodideFuture def eval(code: str) -> Any: ... @@ -44,7 +51,7 @@ class XMLHttpRequest(_JsObject): class Object(_JsObject): @staticmethod - def fromEntries(it: Iterable[tuple[str, Any]]) -> JsProxy: ... + def fromEntries(it: Iterable[JsArray]) -> JsProxy: ... class Array(_JsObject): @staticmethod @@ -77,15 +84,10 @@ class JSON(_JsObject): @staticmethod def parse(a: str) -> JsProxy: ... -class JsElement(JsProxy): - tagName: str - children: list[JsElement] - def appendChild(self, child: JsElement) -> None: ... - class document(_JsObject): - body: JsElement - children: list[JsElement] + body: JsDomElement + children: list[JsDomElement] @staticmethod - def createElement(tagName: str) -> JsElement: ... + def createElement(tagName: str) -> JsDomElement: ... @staticmethod - def appendChild(child: JsElement) -> None: ... + def appendChild(child: JsDomElement) -> None: ... diff --git a/src/py/pyodide/_core.py b/src/py/pyodide/_core.py index 68d908d14..922acb48b 100644 --- a/src/py/pyodide/_core.py +++ b/src/py/pyodide/_core.py @@ -2,38 +2,17 @@ import sys IN_BROWSER = "_pyodide_core" in sys.modules - -if IN_BROWSER: - import _pyodide_core - from _pyodide_core import ( - ConversionError, - JsException, - create_once_callable, - create_proxy, - destroy_proxies, - to_js, - ) - - import _pyodide._core_docs - - _pyodide._core_docs._js_flags = _pyodide_core.js_flags -else: - from _pyodide._core_docs import ( - ConversionError, - JsException, - create_once_callable, - create_proxy, - destroy_proxies, - to_js, - ) - from _pyodide._core_docs import ( + ConversionError, JsArray, JsAsyncGenerator, JsAsyncIterable, JsAsyncIterator, JsBuffer, + JsCallable, + JsDomElement, JsDoubleProxy, + JsException, JsFetchResponse, JsGenerator, JsIterable, @@ -43,29 +22,59 @@ from _pyodide._core_docs import ( JsPromise, JsProxy, JsTypedArray, + create_once_callable, + create_proxy, + destroy_proxies, + to_js, ) +if IN_BROWSER: + import _pyodide_core + + import _pyodide._core_docs + + # This is intentionally opaque to static analysis tools (e.g., mypy) + # + # Note: + # Normally one would handle this by adding type stubs for + # _pyodide_core, but since we already are getting the correct types + # from _core_docs, adding a type stub would introduce a redundancy + # that would be difficult to maintain. + for t in [ + "ConversionError", + "JsException", + "create_once_callable", + "create_proxy", + "destroy_proxies", + "to_js", + ]: + globals()[t] = getattr(_pyodide_core, t) + + _pyodide._core_docs._js_flags = _pyodide_core.js_flags + + __all__ = [ - "JsProxy", - "JsDoubleProxy", - "JsArray", - "JsGenerator", - "JsAsyncGenerator", - "JsIterable", - "JsAsyncIterable", - "JsIterator", - "JsException", - "create_proxy", - "create_once_callable", - "to_js", "ConversionError", - "destroy_proxies", - "JsPromise", - "JsBuffer", - "JsTypedArray", "JsArray", + "JsAsyncGenerator", + "JsAsyncIterable", + "JsAsyncIterator", + "JsBuffer", + "JsDoubleProxy", + "JsException", "JsFetchResponse", + "JsGenerator", + "JsIterable", + "JsIterator", "JsMap", "JsMutableMap", - "JsAsyncIterator", + "JsPromise", + "JsProxy", + "JsDomElement", + "JsCallable", + "JsTypedArray", + "create_once_callable", + "create_proxy", + "destroy_proxies", + "to_js", ] diff --git a/src/py/pyodide/ffi/wrappers.py b/src/py/pyodide/ffi/wrappers.py index 20d1e0116..111d3f899 100644 --- a/src/py/pyodide/ffi/wrappers.py +++ b/src/py/pyodide/ffi/wrappers.py @@ -1,40 +1,59 @@ from collections.abc import Callable -from typing import Any +from typing import Any, Protocol, cast -from .._core import IN_BROWSER, JsProxy, create_once_callable, create_proxy +from .._core import ( + IN_BROWSER, + JsDomElement, + JsProxy, + create_once_callable, + create_proxy, +) if IN_BROWSER: from js import clearInterval, clearTimeout, setInterval, setTimeout -class Destroyable: +class Destroyable(Protocol): def destroy(self): pass -EVENT_LISTENERS: dict[tuple[int, str, Callable[[Any], None]], JsProxy] = {} +# An object with a no-op destroy method so we can do +# +# TIMEOUTS.pop(id, DUMMY_DESTROYABLE).destroy() +# +# and either it gets a real object and calls the real destroy method or it gets +# the fake which does nothing. This is to handle the case where clear_timeout is +# called after the timeout executes. +class DUMMY_DESTROYABLE: + @staticmethod + def destroy(): + pass + + +EVENT_LISTENERS: dict[tuple[int, str, Callable[[Any], None]], Destroyable] = {} def add_event_listener( - elt: JsProxy, event: str, listener: Callable[[Any], None] + elt: JsDomElement, event: str, listener: Callable[[Any], None] ) -> None: """Wrapper for JavaScript's addEventListener() which automatically manages the lifetime of a JsProxy corresponding to the listener param. """ proxy = create_proxy(listener) EVENT_LISTENERS[(elt.js_id, event, listener)] = proxy - elt.addEventListener(event, proxy) # type:ignore[attr-defined] + elt.addEventListener(event, cast(Callable[[Any], None], proxy)) def remove_event_listener( - elt: JsProxy, event: str, listener: Callable[[Any], None] + elt: JsDomElement, event: str, listener: Callable[[Any], None] ) -> None: """Wrapper for JavaScript's removeEventListener() which automatically manages the lifetime of a JsProxy corresponding to the listener param. """ proxy = EVENT_LISTENERS.pop((elt.js_id, event, listener)) - elt.removeEventListener(event, proxy) # type:ignore[attr-defined] - proxy.destroy() # type:ignore[attr-defined] + elt.removeEventListener(event, cast(Callable[[Any], None], proxy)) + proxy.destroy() TIMEOUTS: dict[int, Destroyable] = {} @@ -58,16 +77,6 @@ def set_timeout(callback: Callable[[], None], timeout: int) -> int | JsProxy: return timeout_retval -# An object with a no-op destroy method so we can do -# -# TIMEOUTS.pop(id, DUMMY_DESTROYABLE).destroy() -# -# and either it gets a real object and calls the real destroy method or it gets -# the fake which does nothing. This is to handle the case where clear_timeout is -# called after the timeout executes. -DUMMY_DESTROYABLE = Destroyable() - - def clear_timeout(timeout_retval: int | JsProxy) -> None: """Wrapper for JavaScript's clearTimeout() which automatically manages the lifetime of a JsProxy corresponding to the callback param. @@ -85,7 +94,7 @@ def set_interval(callback: Callable[[], None], interval: int) -> int | JsProxy: of a JsProxy corresponding to the callback param. """ proxy = create_proxy(callback) - interval_retval = setInterval(proxy, interval) + interval_retval = setInterval(cast(Callable[[], None], proxy), interval) id = interval_retval if isinstance(interval_retval, int) else interval_retval.js_id INTERVAL_CALLBACKS[id] = proxy return interval_retval diff --git a/src/py/pyodide/http.py b/src/py/pyodide/http.py index 057324a54..b19af579e 100644 --- a/src/py/pyodide/http.py +++ b/src/py/pyodide/http.py @@ -2,16 +2,21 @@ import json from io import StringIO from typing import IO, Any -from ._core import JsFetchResponse, to_js - -try: - from js import XMLHttpRequest -except ImportError: - pass - -from ._core import IN_BROWSER, JsBuffer, JsException +from ._core import IN_BROWSER, JsBuffer, JsException, JsFetchResponse, to_js from ._package_loader import unpack_buffer +if IN_BROWSER: + from js import Object + + try: + from js import fetch as _jsfetch + except ImportError: + pass + try: + from js import XMLHttpRequest + except ImportError: + pass + __all__ = [ "open_url", "pyfetch", @@ -225,10 +230,6 @@ async def pyfetch(url: str, **kwargs: Any) -> FetchResponse: keyword arguments are passed along as `optional parameters to the fetch API `_. """ - if IN_BROWSER: - from js import Object - from js import fetch as _jsfetch - try: return FetchResponse( url, await _jsfetch(url, to_js(kwargs, dict_converter=Object.fromEntries)) diff --git a/src/tests/test_pyodide.py b/src/tests/test_pyodide.py index 2ade2dcfd..182eb69db 100644 --- a/src/tests/test_pyodide.py +++ b/src/tests/test_pyodide.py @@ -635,7 +635,7 @@ def test_create_proxy(selenium): assert sys.getrefcount(f) == 2 proxy = create_proxy(f) assert sys.getrefcount(f) == 3 - assert proxy() == 7 + assert proxy() == 7 # type:ignore[operator] testAddListener(proxy) assert sys.getrefcount(f) == 3 assert testCallListener() == 7 @@ -677,7 +677,7 @@ def test_create_proxy_roundtrip(selenium): assert o.f.unwrap() is f o.f.destroy() o.f = create_proxy(f, roundtrip=False) - assert o.f is f + assert o.f is f # type: ignore[comparison-overlap] run_js("(o) => { o.f.destroy(); }")(o) diff --git a/src/tests/test_typeconversions.py b/src/tests/test_typeconversions.py index 6b2be7498..8b90a058a 100644 --- a/src/tests/test_typeconversions.py +++ b/src/tests/test_typeconversions.py @@ -585,7 +585,7 @@ def test_wrong_way_track_proxies(selenium): destroy_proxies(proxylist) checkDestroyed(r) with raises(TypeError): - to_js(x, pyproxies=[]) + to_js(x, pyproxies=[]) # type:ignore[call-overload] with raises(TypeError): to_js(x, pyproxies=Object.new()) with raises(ConversionError):