8000 Refactor pyexec (#1318) · trofimander/pyscript@854e9d1 · GitHub
[go: up one dir, main page]
Skip to content

Commit 854e9d1

Browse files
hoodmanehoodmane
authored
Refactor pyexec (pyscript#1318)
This is some refactoring I did on the way towards resolving pyscript#1313. I added a new _run_pyscript Python function which executes the code inside a context manager that sets the display target. We can then return a JS object wrapper directly from Python. I moved the "installation" of the pyscript module to loadInterpreter, and pyimport pyscript_py there and give it a type. This avoids a bunch of creating and deleting of proxies for pyscript_py and allows us to give it a type once and for all. I also did some minor logic cleanup in a few places.
1 parent 689878c commit 854e9d1

18 files changed

+197
-166
lines changed

‎pyscriptjs/directoryManifest.mjs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// This logic split out because it is shared by:
2+
// 1. esbuild.mjs
3+
// 2. Jest setup.ts
4+
5+
import { join } from 'path';
6+
import { opendir, readFile } from 'fs/promises';
7+
8+
/**
9+
* List out everything in a directory, but skip __pycache__ directory. Used to
10+
* list out the directory paths and the [file path, file contents] pairs in the
11+
* Python package. All paths are relative to the directory we are listing. The
12+
* directories are sorted topologically so that a parent directory always
13+
* appears before its children.
14+
*
15+
* This is consumed in main.ts which calls mkdir for each directory and then
16+
* writeFile to create each file.
17+
*
18+
* @param {string} dir The path to the directory we want to list out
19+
* @returns {dirs: string[], files: [string, string][]}
20+
*/
21+
export async function directoryManifest(dir) {
22+
const result = { dirs: [], files: [] };
23+
await _directoryManifestHelper(dir, '.', result);
24+
return result;
25+
}
26+
27+
/**
28+
* Recursive helper function for directoryManifest
29+
*/
30+
async function _directoryManifestHelper(root, dir, result) {
31+
const dirObj = await opendir(join(root, dir));
32+
for await (const d of dirObj) {
33+
const entry = join(dir, d.name);
34+
if (d.isDirectory()) {
35+
if (d.name === '__pycache__') {
36+
continue;
37+
}
38+
result.dirs.push(entry);
39+
await _directoryManifestHelper(root, entry, result);
40+
} else if (d.isFile()) {
41+
result.files.push([entry, await readFile(join(root, entry), { encoding: 'utf-8' })]);
42+
}
43+
}
44+
}

‎pyscriptjs/environment.yml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ channels:
33
- conda-forge
44
- microsoft
55
dependencies:
6-
- python=3.9
6+
- python=3.11
77
- pip
88
- pytest=7
99
- nodejs=16
@@ -18,3 +18,7 @@ dependencies:
1818
- playwright
1919
- pytest-playwright
2020
- pytest-xdist
21+
# We need Pyodide and micropip so we can import them in our Python
22+
# unit tests
23+
- pyodide_py==0.22
24+
- micropip==0.2.2

‎pyscriptjs/esbuild.js renamed to ‎pyscriptjs/esbuild.mjs

Lines changed: 8 additions & 43 deletions
< F987 tr class="diff-line-row">
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
1-
const { build } = require('esbuild');
2-
const { spawn } = require('child_process');
3-
const { join } = require('path');
4-
const { watchFile } = require('fs');
5-
const { cp, lstat, readdir, opendir, readFile } = require('fs/promises');
1+
import { build } from 'esbuild';
2+
import { spawn } from 'child_process';
3+
import { dirname, join } from 'path';
4+
import { watchFile } from 'fs';
5+
import { cp, lstat, readdir } from 'fs/promises';
6+
import { directoryManifest } from './directoryManifest.mjs';
7+
8+
const __dirname = dirname(new URL(import.meta.url).pathname);
69

710
const production = !process.env.NODE_WATCH || process.env.NODE_ENV === 'production';
811

@@ -15,44 +18,6 @@ if (!production) {
1518
copy_targets.push({ src: 'build/*', dest: 'examples/build' });
1619
}
1720

18-
/**
19-
* List out everything in a directory, but skip __pycache__ directory. Used to
20-
* list out the directory paths and the [file path, file contents] pairs in the
21-
* Python package. All paths are relative to the directory we are listing. The
22-
* directories are sorted topologically so that a parent directory always
23-
* appears before its children.
24-
*
25-
* This is consumed in main.ts which calls mkdir for each directory and then
26-
* writeFile to create each file.
27-
*
28-
* @param {string} dir The path to the directory we want to list out
29-
* @returns {dirs: string[], files: [string, string][]}
30-
*/
31-
async function directoryManifest(dir) {
32-
const result = { dirs: [], files: [] };
33-
await _directoryManifestHelper(dir, '.', result);
34-
return result;
35-
}
36-
37-
/**
38-
* Recursive helper function for directoryManifest
39-
*/
40-
async function _directoryManifestHelper(root, dir, result) {
41-
const dirObj = await opendir(join(root, dir));
42-
for await (const d of dirObj) {
43-
const entry = join(dir, d.name);
44-
if (d.isDirectory()) {
45-
if (d.name === '__pycache__') {
46-
continue;
47-
}
48-
result.dirs.push(entry);
49-
await _directoryManifestHelper(root, entry, result);
50-
} else if (d.isFile()) {
51-
result.files.push([entry, await readFile(join(root, entry), { encoding: 'utf-8' })]);
52-
}
53-
}
54-
}
55-
5621
/**
5722
* An esbuild plugin that injects the Pyscript Python package.
5823
*

‎pyscriptjs/jest.config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//jest.config.js
22
module.exports = {
33
preset: 'ts-jest',
4+
setupFilesAfterEnv: ['./tests/unit/setup.ts'],
45
testEnvironment: './jest-environment-jsdom.js',
56
extensionsToTreatAsEsm: ['.ts'],
67
transform: {

‎pyscriptjs/package.json

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
"name": "pyscript",
33
"version": "0.0.1",
44
"scripts": {
5-
"build": "npm run tsc && node esbuild.js",
6-
"dev": "NODE_WATCH=1 node esbuild.js",
5+
"build": "npm run tsc && node esbuild.mjs",
6+
"dev": "NODE_WATCH=1 node esbuild.mjs",
77
"tsc": "tsc --noEmit",
8-
"format:check": "prettier --check './src/**/*.{js,html,ts}'",
9-
"format": "prettier --write './src/**/*.{js,html,ts}'",
10-
"lint": "eslint './src/**/*.{js,html,ts}'",
11-
"lint:fix": "eslint --fix './src/**/*.{js,html,ts}'",
8+
"format:check": "prettier --check './src/**/*.{mjs,js,html,ts}'",
9+
"format": "prettier --write './src/**/*.{mjs,js,html,ts}'",
10+
"lint": "eslint './src/**/*.{mjs,js,html,ts}'",
11+
"lint:fix": "eslint --fix './src/**/*.{mjs,js,html,ts}'",
1212
"xprelint": "npm run format",
1313
"test": "cross-env NODE_OPTIONS=--experimental-vm-modules jest --coverage",
1414
"test:watch": "cross-env NODE_OPTIONS=--experimental-vm-modules jest --watch"

‎pyscriptjs/src/interpreter_client.ts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,23 @@ export class InterpreterClient extends Object {
4545
}
4646

4747
/**
48-
* delegates the code to be run to the underlying interface of
49-
* the remote interpreter.
50-
* Python exceptions are turned into JS exceptions.
51-
* */
48+
* Run user Python code. See also the _run_pyscript docstring.
49+
*
50+
* The result is wrapped in an object to avoid accidentally awaiting a
51+
* Python Task or Future returned as the result of the computation.
52+
*
53+
* @param code the code to run
54+
* @param id The id for the default display target (or undefined if no
55+
* default display target).
56+
* @returns Either:
57+
* 1. An Object of the form {result: the_result} if the result is
58+
* serializable (or transferable), or
59+
* 2. a Synclink Proxy wrapping an object of this if the result is not
60+
* serializable.
61+
*/
5262
// eslint-disable-next-line @typescript-eslint/no-explicit-any
53-
async run(code: string): Promise<{ result: any }> {
54-
return await this._remote.run(code);
63+
async run(code: string, id?: string): Promise<{ result: any }> {
64+
return this._remote.pyscript_py._run_pyscript(code, id);
5565
}
5666

5767
/**

‎pyscriptjs/src/main.ts

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,6 @@ import { RemoteInterpreter } from './remote_interpreter';
2121
import { robustFetch } from './fetch';
2222
import * as Synclink from 'synclink';
2323

24-
// pyscript_package is injected from src/python by bundlePyscriptPythonPlugin in
25-
// esbuild.js
26-
27-
// @ts-ignore
28-
import python_package from 'pyscript_python_package.esbuild_injected.json';
29-
30-
declare const python_package: { dirs: string[]; files: [string, string] };
31-
3224
const logger = getLogger('pyscript/main');
3325

3426
/**
@@ -271,17 +263,6 @@ export class PyScriptApp {
271263
// XXX: maybe the following calls could be parallelized, instead of
272264
// await()ing immediately. For now I'm using await to be 100%
273265
// compatible with the old behavior.
274-
logger.info('importing pyscript');
275-
276-
// Write pyscript package into file system
277-
for (const dir of python_package.dirs) {
278-
await interpreter._remote.FS.mkdir('/home/pyodide/' + dir);
279-
}
280-
for (const [path, value] of python_package.files) {
281-
await interpreter._remote.FS.writeFile('/home/pyodide/' + path, value);
282-
}
283-
//Refresh the module cache so Python consistently finds pyscript module
284-
await interpreter._remote.invalidate_module_path_cache();
285266

286267
// inject `define_custom_element` and showWarning it into the PyScript
287268
// module scope
@@ -290,11 +271,7 @@ export class PyScriptApp {
290271
// await interpreter._remote.setHandler('showWarning', Synclink.proxy(showWarning));
291272
interpreter._unwrapped_remote.setHandler('define_custom_element', define_custom_element);
292273
interpreter._unwrapped_remote.setHandler('showWarning', showWarning);
293-
const pyscript_module = (await interpreter.pyimport('pyscript')) as Synclink.Remote<
294-
PyProxy & { _set_version_info(string): void }
295-
>;
296-
await pyscript_module._set_version_info(version);
297-
await pyscript_module.destroy();
274+
await interpreter._remote.pyscript_py._set_version_info(version);
298275

299276
// import some carefully selected names into the global namespace
300277
await interpreter.run(`

‎pyscriptjs/src/pyexec.ts

10000 Lines changed: 23 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ import { getLogger } from './logger';
22
import { ensureUniqueId } from './utils';
33
import { UserError, ErrorCode } from './exceptions';
44
import { InterpreterClient } from './interpreter_client';
5-
import type { PyProxyCallable, PyProxy } from 'pyodide';
6-
import type { Remote } from 'synclink';
5+
import type { PyProxyCallable } from 'pyodide';
76

87
const logger = getLogger('pyexec');
98

@@ -13,39 +12,30 @@ export async function pyExec(
1312
outElem: HTMLElement,
1413
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1514
): Promise<{ result: any }> {
16-
const pyscript_py = (await interpreter.pyimport('pyscript')) as Remote<
17-
PyProxy & {
18-
set_current_display_target(id: string): void;
19-
uses_top_level_await(code: string): boolean;
20-
}
21-
>;
2215
ensureUniqueId(outElem);
23-
await pyscript_py.set_current_display_target(outElem.id);
16+
if (await interpreter._remote.pyscript_py.uses_top_level_await(pysrc)) {
17+
const err = new UserError(
18+
ErrorCode.TOP_LEVEL_AWAIT,
19+
'The use of top-level "await", "async for", and ' +
20+
'"async with" has been removed.' +
21+
'\nPlease write a coroutine containing ' +
22+
'your code and schedule it using asyncio.ensure_future() or similar.' +
23+
'\nSee https://docs.pyscript.net/latest/guides/asyncio.html for more information.',
24+
);
25+
displayPyException(err, outElem);
26+
return { result: undefined };
27+
}
28+
2429
try {
25-
try {
26-
if (await pyscript_py.uses_top_level_await(pysrc)) {
27-
throw new UserError(
28-
ErrorCode.TOP_LEVEL_AWAIT,
29-
'The use of top-level "await", "async for", and ' +
30-
'"async with" has been removed.' +
31-
'\nPlease write a coroutine containing ' +
32-
'your code and schedule it using asyncio.ensure_future() or similar.' +
33-
'\nSee https://docs.pyscript.net/latest/guides/asyncio.html for more information.',
34-
);
35-
}
36-
return await interpreter.run(pysrc);
37-
} catch (e) {
38-
const err = e as Error;
39-
// XXX: currently we display exceptions in the same position as
40-
// the output. But we probably need a better way to do that,
41-
// e.g. allowing plugins to intercept exceptions and display them
42-
// in a configurable way.
43-
displayPyException(err, outElem);
44-
return { result: undefined };
45-
}
46-
} finally {
47-
await pyscript_py.set_current_display_target(undefined);
48-
await pyscript_py.destroy();
30+
return await interpreter.run(pysrc, outElem.id);
31+
} catch (e) {
32+
const err = e as Error;
33+
// XXX: currently we display exceptions in the same position as
34+
// the output. But we probably need a better way to do that,
35+
// e.g. allowing plugins to intercept exceptions and display them
36+
// in a configurable way.
37+
displayPyException(err, outElem);
38+
return { result: undefined };
4939
}
5040
}
5141

‎pyscriptjs/src/python/pyscript/__init__.py

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,17 @@
66
import re
77
import time
88
from collections import namedtuple
9+
from contextlib import contextmanager
910
from textwrap import dedent
1011

1112
import js
1213

1314
try:
14-
from pyodide.ffi import create_proxy
15+
from pyodide.code import eval_code
16+
from pyodide.ffi import JsProxy, create_proxy
1517
except ImportError:
16-
from pyodide import create_proxy
18+
from pyodide import JsProxy, create_proxy, eval_code
19+
1720

1821
loop = asyncio.get_event_loop()
1922

@@ -188,8 +191,13 @@ def write(element_id, value, append=False, exec_id=0):
188191
)
189192

190193

191-
def set_current_display_target(target_id):
194+
@contextmanager
195+
def _display_target(target_id):
192196
get_current_display_target._id = target_id
197+
try:
198+
yield
199+
finally:
200+
get_current_display_target._id = None
193201

194202

195203
def get_current_display_target():
@@ -200,17 +208,14 @@ def get_current_display_target():
200208

201209

202210
def display(*values, target=None, append=True):
203-
default_target = get_current_display_target()
204-
if default_target is None and target is None:
211+
if target is None:
212+
target = get_current_display_target()
213+
if target is None:
205214
raise Exception(
206215
"Implicit target not allowed here. Please use display(..., target=...)"
207216
)
208-
if target is not None:
209-
for v in values:
210-
Element(target).write(v, append=append)
211-
else:
212-
for v in values:
213-
Element(default_target).write(v, append=append)
217+
for v in values:
218+
Element(target).write(v, append=append)
214219

215220

216221
class Element:
@@ -702,3 +707,32 @@ def deprecate(name, obj, instead):
702707
"Please use <code>pyscript</code> instead."
703708
)
704709
ns["PyScript"] = DeprecatedGlobal("PyScript", PyScript, message)
710+
711+
712+
def _run_pyscript(code: str, id: str = None) -> JsProxy:
713+
"""Execute user code inside context managers.
714+
715+
Uses the __main__ global namespace.
716+
717+
The output is wrapped inside a JavaScript object since an object is not
718+
thenable. This is so we do not accidentally `await` the result of the python
719+
execution, even if it's awaitable (Future, Task, etc.)
720+
721+
Parameters
722+
----------
723+
code :
724+
The code to run
725+
726+
id :
727+
The id for the default display target (or None if no default display target).
728+
729+
Returns
730+
-------
731+
A Js Object of the form {result: the_result}
732+
"""
733+
import __main__
734+
735+
with _display_target(id):
736+
result = eval_code(code, globals=__main__.__dict__)
737+
738+
return js.Object.new(result=result)

0 commit comments

Comments
 (0)
0