8000 kill unwrapped_remote (#1490) · pyscript/pyscript@17d16b9 · GitHub
[go: up one dir, main page]
Skip to content

Commit 17d16b9

Browse files
madhur-tandonmadhur-tandon
authored
kill unwrapped_remote (#1490)
* kill unwrapped_remote * linting * don't use callKwargs for python plugins * fix tests and improve types
1 parent 8e86daa commit 17d16b9

File tree

5 files changed

+71
-46
lines changed

5 files changed

+71
-46
lines changed

‎pyscriptjs/src/interpreter_client.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,24 +13,17 @@ InterpreterClient class is responsible to request code execution
1313
*/
1414
export class InterpreterClient extends Object {
1515
_remote: Synclink.Remote<RemoteInterpreter>;
16-
_unwrapped_remote: RemoteInterpreter;
1716
config: AppConfig;
1817
/**
1918
* global symbols table for the underlying interface.
2019
* */
2120
globals: Synclink.Remote<PyProxyDict>;
2221
stdio: Stdio;
2322

24-
constructor(
25-
config: AppConfig,
26-
stdio: Stdio,
27-
remote: Synclink.Remote<RemoteInterpreter>,
28-
unwrapped_remote: RemoteInterpreter,
29-
) {
23+
constructor(config: AppConfig, stdio: Stdio, remote: Synclink.Remote<RemoteInterpreter>) {
3024
super();
3125
this.config = config;
3226
this._remote = remote;
33-
this._unwrapped_remote = unwrapped_remote;
3427
this.stdio = stdio;
3528
}
3629

‎pyscriptjs/src/main.ts

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ export class PyScriptApp {
212212
*/
213213
const interpreterURL = interpreter_cfg.src;
214214
await import(interpreterURL);
215-
return { remote_interpreter, wrapped_remote_interpreter };
215+
return wrapped_remote_interpreter;
216216
}
217217

218218
async _startInterpreter_worker(interpreter_cfg: InterpreterConfig) {
@@ -222,8 +222,7 @@ export class PyScriptApp {
222222
const worker = new Worker(base_url + '/interpreter_worker.js');
223223
const worker_initialize: any = Synclink.wrap(worker);
224224
const wrapped_remote_interpreter = await worker_initialize(interpreter_cfg);
225-
const remote_interpreter = undefined; // this is _unwrapped_remote
226-
return { remote_interpreter, wrapped_remote_interpreter };
225+
return wrapped_remote_interpreter;
227226
}
228227

229228
// lifecycle (4)
@@ -238,19 +237,17 @@ export class PyScriptApp {
238237
}
239238

240239
const cfg = this.config.interpreters[0];
241-
let x;
240+
let wrapped_remote_interpreter;
242241
if (this.config.execution_thread == 'worker') {
243-
x = await this._startInterpreter_worker(cfg);
242+
wrapped_remote_interpreter = await this._startInterpreter_worker(cfg);
244243
} else {
245-
x = await this._startInterpreter_main(cfg);
244+
wrapped_remote_interpreter = await this._startInterpreter_main(cfg);
246245
}
247-
const { remote_interpreter, wrapped_remote_interpreter } = x;
248246

249247
this.interpreter = new InterpreterClient(
250248
this.config,
251249
this._stdioMultiplexer,
252250
wrapped_remote_interpreter as Synclink.Remote<RemoteInterpreter>,
253-
remote_interpreter,
254251
);
255252
await this.afterInterpreterLoad(this.interpreter);
256253
}
@@ -413,11 +410,9 @@ export class PyScriptApp {
413410
// TODO: This is very specific to Pyodide API and will not work for other interpreters,
414411
// when we add support for other interpreters we will need to move this to the
415412
// interpreter API level and allow each one to implement it in its own way
416-
417-
// eventually replace with interpreter.pyimport(modulename);
418-
const module = interpreter._unwrapped_remote.pyimport(modulename);
413+
const module = await interpreter.pyimport(modulename);
419414
if (typeof (await module.plugin) !== 'undefined') {
420-
const py_plugin = module.plugin as PythonPlugin;
415+
const py_plugin = (await module.plugin) as PythonPlugin;
421416
py_plugin.init(this);
422417
this.plugins.addPythonPlugin(py_plugin);
423418
} else {

‎pyscriptjs/src/plugin.ts

Lines changed: 62 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@ import { UserError, ErrorCode } from './exceptions';
44
import { getLogger } from './logger';
55
import { make_PyScript } from './components/pyscript';
66
import { InterpreterClient } from './interpreter_client';
7+
import { make_PyRepl } from './components/pyrepl';
78

89
const logger = getLogger('plugin');
910
type PyScriptTag = InstanceType<ReturnType<typeof make_PyScript>>;
11+
type PyReplTag = InstanceType<ReturnType<typeof make_PyRepl>>;
1012

1113
export class Plugin {
1214
/** Validate the configuration of the plugin and handle default values.
@@ -84,7 +86,12 @@ export class Plugin {
8486
* @param options.outEl The element that the result of the REPL evaluation will be output to.
8587
* @param options.pyReplTag The <py-repl> HTML tag the originated the evaluation
8688
*/
87-
beforePyReplExec(options: { interpreter: InterpreterClient; src: string; outEl: HTMLElement; pyReplTag: any }) {
89+
beforePyReplExec(options: {
90+
interpreter: InterpreterClient;
91+
src: string;
92+
outEl: HTMLElement;
93+
pyReplTag: PyReplTag;
94+
}) {
8895
/* empty */
8996
}
9097

@@ -100,7 +107,7 @@ export class Plugin {
100107
interpreter: InterpreterClient;
101108
src: string;
102109
outEl: HTMLElement;
103-
pyReplTag: HTMLElement;
110+
pyReplTag: PyReplTag;
104111
result: any;
105112
}) {
106113
/* empty */
@@ -126,10 +133,26 @@ export type PythonPlugin = {
126133
configure?: (config: AppConfig) => Promise<void>;
127134
afterSetup?: (interpreter: InterpreterClient) => Promise<void>;
128135
afterStartup?: (interpreter: InterpreterClient) => Promise<void>;
129-
beforePyScriptExec?: { callKwargs: (options: any) => Promise<void> };
130-
afterPyScriptExec?: { callKwargs: (options: any) => Promise<void> };
131-
beforePyReplExec?: { callKwargs: (options: any) => Promise<void> };
132-
afterPyReplExec?: { callKwargs: (options: any) => Promise<void> };
136+
beforePyScriptExec?: (interpreter: InterpreterClient, src: string, pyScriptTag: PyScriptTag) => Promise<void>;
137+
afterPyScriptExec?: (
138+
interpreter: InterpreterClient,
139+
src: string,
140+
pyScriptTag: PyScriptTag,
141+
result: any,
142+
) => Promise<void>;
143+
beforePyReplExec?: (
144+
interpreter: InterpreterClient,
145+
src: string,
146+
outEl: HTMLElement,
147+
pyReplTag: PyReplTag,
148+
) => Promise<void>;
149+
afterPyReplExec?: (
150+
interpreter: InterpreterClient,
151+
src: string,
152+
outEl: HTMLElement,
153+
pyReplTag: PyReplTag,
154+
result: any,
155+
) => Promise<void>;
133156
onUserError?: (error: UserError) => Promise<void>;
134157
};
135158

@@ -188,7 +211,9 @@ export class PluginManager {
188211

189212
async beforePyScriptExec(options: { interpreter: InterpreterClient; src: string; pyScriptTag: PyScriptTag }) {
190213
await Promise.all(this._plugins.map(p => p.beforePyScriptExec?.(options)));
191-
await Promise.all(this._pythonPlugins.map(p => p.beforePyScriptExec.callKwargs(options)));
214+
await Promise.all(
215+
this._pythonPlugins.map(p => p.beforePyScriptExec?.(options.interpreter, options.src, options.pyScriptTag)),
216+
);
192217
}
193218

194219
async afterPyScriptExec(options: {
@@ -198,22 +223,47 @@ export class PluginManager {
198223
result: any;
199224
}) {
200225
await Promise.all(this._plugins.map(p => p.afterPyScriptExec?.(options)));
201-
await Promise.all(this._pythonPlugins.map(p => p.afterPyScriptExec.callKwargs(options)));
226+
await Promise.all(
227+
this._pythonPlugins.map(
228+
p => p.afterPyScriptExec?.(options.interpreter, options.src, options.pyScriptTag, options.result),
229+
),
230+
);
202231
}
203232

204233
async beforePyReplExec(options: {
205234
interpreter: InterpreterClient;
206235
src: string;
207236
outEl: HTMLElement;
208-
pyReplTag: any;
237+
pyReplTag: PyReplTag;
209238
}) {
210239
await Promise.all(this._plugins.map(p => p.beforePyReplExec?.(options)));
211-
await Promise.all(this._pythonPlugins.map(p => p.beforePyReplExec.callKwargs(options)));
240+
await Promise.all(
241+
this._pythonPlugins.map(
242+
p => p.beforePyReplExec?.(options.interpreter, options.src, options.outEl, options.pyReplTag),
243+
),
244+
);
212245
}
213246

214-
async afterPyReplExec(options: { interpreter: InterpreterClient; src: string; outEl; pyReplTag; result }) {
247+
async afterPyReplExec(options: {
248+
interpreter: InterpreterClient;
249+
src: string;
250+
outEl: HTMLElement;
251+
pyReplTag: PyReplTag;
252+
result: any;
253+
}) {
215254
await Promise.all(this._plugins.map(p => p.afterPyReplExec?.(options)));
216-
await Promise.all(this._pythonPlugins.map(p => p.afterPyReplExec.callKwargs(options)));
255+
await Promise.all(
256+
this._pythonPlugins.map(
257+
p =>
258+
p.afterPyReplExec?.(
259+
options.interpreter,
260+
options.src,
261+
options.outEl,
262+
options.pyReplTag,
263+
options.result,
264+
),
265+
),
266+
);
217267
}
218268

219269
async onUserError(error: UserError) {

‎pyscriptjs/tests/integration/test_plugins.py

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,10 @@ class ExecTestLogger(Plugin):
6363
async def beforePyScriptExec(self, interpreter, src, pyScriptTag):
6464
console.log(f'beforePyScriptExec called')
6565
console.log(f'before_src:{src}')
66-
console.log(f'before_id:{pyScriptTag.id}')
6766
6867
async def afterPyScriptExec(self, interpreter, src, pyScriptTag, result):
6968
console.log(f'afterPyScriptExec called')
7069
console.log(f'after_src:{src}')
71-
console.log(f'after_id:{pyScriptTag.id}')
7270
console.log(f'result:{result}')
7371
7472
@@ -88,12 +86,10 @@ class PyReplTestLogger(Plugin):
8886
def beforePyReplExec(self, interpreter, src, outEl, pyReplTag):
8987
console.log(f'beforePyReplExec called')
9088
console.log(f'before_src:{src}')
91-
console.log(f'before_id:{pyReplTag.id}')
9289
9390
def afterPyReplExec(self, interpreter, src, outEl, pyReplTag, result):
9491
console.log(f'afterPyReplExec called')
9592
console.log(f'after_src:{src}')
96-
console.log(f'after_id:{pyReplTag.id}')
9793
console.log(f'result:{result}')
9894
9995
@@ -261,9 +257,7 @@ def test_pyscript_exec_hooks(self):
261257
# These could be made better with a utility function that found log lines
262258
# that match a filter function, or start with something
263259
assert "before_src:x=2; x" in log_lines
264-
assert "before_id:pyid" in log_lines
265260
assert "after_src:x=2; x" in log_lines
266-
assert "after_id:pyid" in log_lines
267261
assert "result:2" in log_lines
268262

269263
@skip_worker("FIXME: relative paths")
@@ -286,9 +280,7 @@ def test_pyrepl_exec_hooks(self):
286280
# These could be made better with a utility function that found log lines
287281
# that match a filter function, or start with something
288282
assert "before_src:x=2; x" in log_lines
289-
assert "before_id:pyid" in log_lines
290283
assert "after_src:x=2; x" in log_lines
291-
assert "after_id:pyid" in log_lines
292284
assert "result:2" in log_lines
293285

294286
@skip_worker("FIXME: relative paths")

‎pyscriptjs/tests/unit/pyodide.test.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ describe('RemoteInterpreter', () => {
2929
config,
3030
stdio,
3131
wrapped_remote_interpreter as Synclink.Remote<RemoteInterpreter>,
32-
remote_interpreter,
3332
);
3433

3534
/**
@@ -69,10 +68,6 @@ describe('RemoteInterpreter', () => {
6968
expect(interpreter).toBeInstanceOf(InterpreterClient);
7069
});
7170

72-
it('should check if interpreter is an instance of RemoteInterpreter', async () => {
73-
expect(interpreter._unwrapped_remote).toBeInstanceOf(RemoteInterpreter);
74-
});
75-
7671
it('should check if interpreter can run python code asynchronously', async () => {
7772
expect((await interpreter.run('2+3')).result).toBe(5);
7873
});
@@ -85,7 +80,7 @@ describe('RemoteInterpreter', () => {
8580

8681
it('should check if interpreter is able to load a package', async () => {
8782
stdio.reset();
88-
await interpreter._unwrapped_remote.loadPackage('numpy');
83+
await interpreter._remote.loadPackage('numpy');
8984
await interpreter.run('import numpy as np');
9085
await interpreter.run('x = np.ones((10,))');
9186
await interpreter.run('print(x)');

0 commit comments

Comments
 (0)
0