From 6a4423356283a5d644c0e144f59570be0d1ba74c Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Fri, 22 Jan 2021 03:28:22 -0800 Subject: [PATCH] Don't automatically copy python objects into javascript (#1152) * Don't automatically copy python objects into javascript, add new PyProxy.shallowCopyToJavascript and deepCopyToJavascript apis * Lint * Adjust conftest to automatically deep_copy results of run / run_async * Update conftest again * Fix deep/shallowCopyToJavascript * Fix shallowCopyToJavascript and deepCopyToJavascript to return js object not hiwire id * Fix the remaining tests (hopefully) * Lint * Fix more tests * Fix some numpy tests * Lint * Fix more tests * Lint * Temporarily dummy out setLineDash since it breaks test --- conftest.py | 24 +++++++++++++-- packages/matplotlib/src/wasm_backend.py | 2 +- packages/matplotlib/test_matplotlib.py | 8 ++--- packages/numpy/test_numpy.py | 39 ++++++++++++++++++------- src/core/pyproxy.c | 12 ++++++++ src/core/python2js.c | 2 +- src/core/python2js.h | 10 ++++++- src/pyodide.js | 2 +- src/tests/test_jsproxy.py | 8 ++--- src/tests/test_pyproxy.py | 2 ++ src/tests/test_python.py | 2 +- src/tests/test_typeconversions.py | 8 ++--- 12 files changed, 90 insertions(+), 29 deletions(-) diff --git a/conftest.py b/conftest.py index 30ce01ed7..fb4acc38d 100644 --- a/conftest.py +++ b/conftest.py @@ -95,10 +95,30 @@ class SeleniumWrapper: self.driver.execute_script("window.logs = []") def run(self, code): - return self.run_js("return pyodide.runPython({!r})".format(code)) + return self.run_js( + f""" + let result = pyodide.runPython({code!r}); + if(result && result.deepCopyToJavascript){{ + let converted_result = result.deepCopyToJavascript(); + result.destroy(); + return converted_result; + }} + return result; + """ + ) def run_async(self, code): - return self.run_js("return pyodide.runPythonAsync({!r})".format(code)) + return self.run_js( + f""" + let result = await pyodide.runPythonAsync({code!r}); + if(result && result.deepCopyToJavascript){{ + let converted_result = result.deepCopyToJavascript(); + result.destroy(); + return converted_result; + }} + return result; + """ + ) def run_js(self, code): if isinstance(code, str) and code.startswith("\n"): diff --git a/packages/matplotlib/src/wasm_backend.py b/packages/matplotlib/src/wasm_backend.py index a206b3c72..e8a16279d 100644 --- a/packages/matplotlib/src/wasm_backend.py +++ b/packages/matplotlib/src/wasm_backend.py @@ -190,7 +190,7 @@ class FigureCanvasWasm(backend_agg.FigureCanvasAgg): rubberband.addEventListener("keydown", self.onkeydown) context = rubberband.getContext("2d") context.strokeStyle = "#000000" - context.setLineDash([2, 2]) + # context.setLineDash([2, 2]) canvas_div.appendChild(rubberband) div.appendChild(canvas_div) diff --git a/packages/matplotlib/test_matplotlib.py b/packages/matplotlib/test_matplotlib.py index b37e16fa1..243575c80 100644 --- a/packages/matplotlib/test_matplotlib.py +++ b/packages/matplotlib/test_matplotlib.py @@ -2,15 +2,15 @@ def test_matplotlib(selenium_standalone): selenium = selenium_standalone selenium.load_package("matplotlib") selenium.run("from matplotlib import pyplot as plt") - selenium.run("plt.figure()") - selenium.run("plt.plot([1,2,3])") + selenium.run("plt.figure(); pass") + selenium.run("x = plt.plot([1,2,3])") selenium.run("plt.show()") def test_svg(selenium): selenium.load_package("matplotlib") selenium.run("from matplotlib import pyplot as plt") - selenium.run("plt.figure()") + selenium.run("plt.figure(); pass") selenium.run("x = plt.plot([1,2,3])") selenium.run("import io") selenium.run("fd = io.BytesIO()") @@ -23,7 +23,7 @@ def test_svg(selenium): def test_pdf(selenium): selenium.load_package("matplotlib") selenium.run("from matplotlib import pyplot as plt") - selenium.run("plt.figure()") + selenium.run("plt.figure(); pass") selenium.run("x = plt.plot([1,2,3])") selenium.run("import io") selenium.run("fd = io.BytesIO()") diff --git a/packages/numpy/test_numpy.py b/packages/numpy/test_numpy.py index f4c1d3c1c..c46a1cb99 100644 --- a/packages/numpy/test_numpy.py +++ b/packages/numpy/test_numpy.py @@ -2,11 +2,17 @@ def test_numpy(selenium): selenium.load_package("numpy") selenium.run("import numpy") selenium.run("x = numpy.ones((32, 64))") - assert selenium.run_js("return pyodide.pyimport('x').length == 32") + assert selenium.run_js( + "return pyodide.pyimport('x').deepCopyToJavascript().length == 32" + ) for i in range(32): - assert selenium.run_js(f"return pyodide.pyimport('x')[{i}].length == 64") + assert selenium.run_js( + f"return pyodide.pyimport('x').deepCopyToJavascript()[{i}].length == 64" + ) for j in range(64): - assert selenium.run_js(f"return pyodide.pyimport('x')[{i}][{j}] == 1") + assert selenium.run_js( + f"return pyodide.pyimport('x').deepCopyToJavascript()[{i}][{j}] == 1" + ) def test_typed_arrays(selenium): @@ -48,7 +54,9 @@ def test_python2js_numpy_dtype(selenium_standalone): for j in range(2): for k in range(2): assert ( - selenium.run_js(f"return pyodide.pyimport('x')[{i}][{j}][{k}]") + selenium.run_js( + f"return pyodide.pyimport('x').deepCopyToJavascript()[{i}][{j}][{k}]" + ) == expected_result[i][j][k] ) @@ -74,7 +82,7 @@ def test_python2js_numpy_dtype(selenium_standalone): ) assert_equal() classname = selenium.run_js( - "return pyodide.pyimport('x')[0][0].constructor.name" + "return pyodide.pyimport('x').deepCopyToJavascript()[0][0].constructor.name" ) if order == "C" and dtype not in ("uint64", "int64"): # Here we expect a TypedArray subclass, such as Uint8Array, but @@ -90,7 +98,7 @@ def test_python2js_numpy_dtype(selenium_standalone): ) assert_equal() classname = selenium.run_js( - "return pyodide.pyimport('x')[0][0].constructor.name" + "return pyodide.pyimport('x').deepCopyToJavascript()[0][0].constructor.name" ) if order == "C" and dtype in ("int8", "uint8"): # Here we expect a TypedArray subclass, such as Uint8Array, but @@ -104,9 +112,18 @@ def test_python2js_numpy_dtype(selenium_standalone): assert selenium.run("np.array([True, False])") == [True, False] selenium.run("x = np.array([['string1', 'string2'], ['string3', 'string4']])") - assert selenium.run_js("return pyodide.pyimport('x').length") == 2 - assert selenium.run_js("return pyodide.pyimport('x')[0][0]") == "string1" - assert selenium.run_js("return pyodide.pyimport('x')[1][1]") == "string4" + assert ( + selenium.run_js("return pyodide.pyimport('x').deepCopyToJavascript().length") + == 2 + ) + assert ( + selenium.run_js("return pyodide.pyimport('x').deepCopyToJavascript()[0][0]") + == "string1" + ) + assert ( + selenium.run_js("return pyodide.pyimport('x').deepCopyToJavascript()[1][1]") + == "string4" + ) def test_py2js_buffer_clear_error_flag(selenium): @@ -176,7 +193,9 @@ def test_runpythonasync_numpy(selenium_standalone): """ ) for i in range(5): - assert selenium_standalone.run_js(f"return pyodide.pyimport('x')[{i}] == 0") + assert selenium_standalone.run_js( + f"return pyodide.pyimport('x').deepCopyToJavascript()[{i}] == 0" + ) def test_runwebworker_numpy(selenium_standalone): diff --git a/src/core/pyproxy.c b/src/core/pyproxy.c index d12c82e5f..3b467ccea 100644 --- a/src/core/pyproxy.c +++ b/src/core/pyproxy.c @@ -223,6 +223,18 @@ EM_JS(int, pyproxy_init, (), { Module.hiwire.decref(idresult); return jsresult; }, + shallowCopyToJavascript : function(){ + let idresult = _python2js_with_depth(_getPtr(this), depth); + let result = Module.hiwire.get_value(idresult); + Module.hiwire.decref(idresult); + return result; + }, + deepCopyToJavascript : function(depth = -1){ + let idresult = _python2js_with_depth(_getPtr(this), depth); + let result = Module.hiwire.get_value(idresult); + Module.hiwire.decref(idresult); + return result; + }, }; let ignoredTargetFields = ["name", "length"]; diff --git a/src/core/python2js.c b/src/core/python2js.c index 71b8d5388..f557f42a6 100644 --- a/src/core/python2js.c +++ b/src/core/python2js.c @@ -342,7 +342,7 @@ JsRef python2js(PyObject* x) { PyObject* map = PyDict_New(); - JsRef result = _python2js_cache(x, map, -1); + JsRef result = _python2js_cache(x, map, 0); Py_DECREF(map); if (result == NULL) { diff --git a/src/core/python2js.h b/src/core/python2js.h index cd3eb89da..8371112b9 100644 --- a/src/core/python2js.h +++ b/src/core/python2js.h @@ -16,13 +16,21 @@ void pythonexc2js(); /** Convert a Python object to a Javascript object. - * \param The Python object + * \param x The Python object * \return The Javascript object -- might be an Error object in the case of an * exception. */ JsRef python2js(PyObject* x); +/** Convert a Python object to a Javascript object, copying standard collections + * into javascript down to specified depth \param x The Python object \param + * depth The maximum depth to copy \return The Javascript object -- might be an + * Error object in the case of an exception. + */ +JsRef +python2js_with_depth(PyObject* x, int depth); + /** Set up the global state for this module. */ int diff --git a/src/pyodide.js b/src/pyodide.js index 32ae73630..e7201908b 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -362,7 +362,7 @@ globalThis.languagePluginLoader = new Promise((resolve, reject) => { // clang-format off Module.loadPackagesFromImports = async function(code, messageCallback, errorCallback) { - let imports = Module.pyodide_py.find_imports(code); + let imports = Module.pyodide_py.find_imports(code).deepCopyToJavascript(); if (imports.length === 0) { return; } diff --git a/src/tests/test_jsproxy.py b/src/tests/test_jsproxy.py index e41753f34..94b2a69a3 100644 --- a/src/tests/test_jsproxy.py +++ b/src/tests/test_jsproxy.py @@ -12,7 +12,7 @@ def test_jsproxy_dir(selenium): from js import a from js import b [dir(a), dir(b)] - `); + `).deepCopyToJavascript(); """ ) jsproxy_items = set( @@ -49,7 +49,7 @@ def test_jsproxy_getattr(selenium): return pyodide.runPython(` from js import a [ a.x, a.y, a.typeof ] - `); + `).deepCopyToJavascript(); """ ) == [2, "9", "object"] @@ -195,7 +195,7 @@ def test_jsproxy_call(selenium): from js import f [f(*range(n)) for n in range(10)] ` - ); + ).deepCopyToJavascript(); """ ) == list(range(10)) @@ -379,7 +379,7 @@ def test_mount_object(selenium): import b result += [a.s, dir(a), dir(b)] result - `) + `).deepCopyToJavascript() """ ) assert result[:3] == ["x1", "x2", 3] diff --git a/src/tests/test_pyproxy.py b/src/tests/test_pyproxy.py index fb3f5b71c..532f53598 100644 --- a/src/tests/test_pyproxy.py +++ b/src/tests/test_pyproxy.py @@ -55,6 +55,8 @@ def test_pyproxy(selenium): "apply", "destroy", "$$", + "deepCopyToJavascript", + "shallowCopyToJavascript", ] ) assert selenium.run("hasattr(f, 'baz')") diff --git a/src/tests/test_python.py b/src/tests/test_python.py index 9713c818a..c19e59f83 100644 --- a/src/tests/test_python.py +++ b/src/tests/test_python.py @@ -172,5 +172,5 @@ def test_unknown_attribute(selenium): def test_run_python_debug(selenium): assert selenium.run_js("return pyodide._module.runPythonDebug('1+1');") == 2 assert selenium.run_js( - "return pyodide._module.runPythonDebug('[x*x + 1 for x in range(4)]');" + "return pyodide._module.runPythonDebug('[x*x + 1 for x in range(4)]').deepCopyToJavascript();" ) == [1, 2, 5, 10] diff --git a/src/tests/test_typeconversions.py b/src/tests/test_typeconversions.py index d475c9749..55f98b7d3 100644 --- a/src/tests/test_typeconversions.py +++ b/src/tests/test_typeconversions.py @@ -22,14 +22,14 @@ def test_python2js(selenium): ) assert selenium.run_js( """ - let x = pyodide.runPython("[1, 2, 3]"); + let x = pyodide.runPython("[1, 2, 3]").deepCopyToJavascript(); return ((x instanceof window.Array) && (x.length === 3) && (x[0] == 1) && (x[1] == 2) && (x[2] == 3)) """ ) assert selenium.run_js( """ - let x = pyodide.runPython("{42: 64}"); + let x = pyodide.runPython("{42: 64}").deepCopyToJavascript(); return (typeof x === "object") && (x[42] === 64) """ ) @@ -230,7 +230,7 @@ def test_recursive_list_to_js(selenium_standalone): x.append(x) """ ) - selenium_standalone.run_js("x = pyodide.pyimport('x');") + selenium_standalone.run_js("x = pyodide.pyimport('x').deepCopyToJavascript();") def test_recursive_dict_to_js(selenium_standalone): @@ -240,7 +240,7 @@ def test_recursive_dict_to_js(selenium_standalone): x[0] = x """ ) - selenium_standalone.run_js("x = pyodide.pyimport('x');") + selenium_standalone.run_js("x = pyodide.pyimport('x').deepCopyToJavascript();") def test_list_from_js(selenium):