From dc7865e508749c17b792fa49c19508b525b6b61a Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Tue, 7 Jan 2025 11:20:11 +0100 Subject: [PATCH 1/7] Allow to reorder engine prioritization with `engines` key in _quarto.yml --- src/execute/engine.ts | 47 +++++++++++++++++++++++++++++++--- src/execute/julia.ts | 25 ++++++++++++++---- src/execute/jupyter/jupyter.ts | 6 +++-- src/execute/jupyter/percent.ts | 9 ++++--- 4 files changed, 72 insertions(+), 15 deletions(-) diff --git a/src/execute/engine.ts b/src/execute/engine.ts index 245d66ccfd..1c869a6f6b 100644 --- a/src/execute/engine.ts +++ b/src/execute/engine.ts @@ -96,7 +96,11 @@ export function engineValidExtensions(): string[] { ); } -export function markdownExecutionEngine(markdown: string, flags?: RenderFlags) { +export function markdownExecutionEngine( + markdown: string, + reorderedEngines: Map, + flags?: RenderFlags, +) { // read yaml and see if the engine is declared in yaml // (note that if the file were a non text-file like ipynb // it would have already been claimed via extension) @@ -106,7 +110,7 @@ export function markdownExecutionEngine(markdown: string, flags?: RenderFlags) { if (yaml) { // merge in command line fags yaml = mergeConfigs(yaml, flags?.metadata); - for (const [_, engine] of kEngines) { + for (const [_, engine] of reorderedEngines) { if (yaml[engine.name]) { return engine; } @@ -123,7 +127,7 @@ export function markdownExecutionEngine(markdown: string, flags?: RenderFlags) { // see if there is an engine that claims this language for (const language of languages) { - for (const [_, engine] of kEngines) { + for (const [_, engine] of reorderedEngines) { if (engine.claimsLanguage(language)) { return engine; } @@ -143,6 +147,38 @@ export function markdownExecutionEngine(markdown: string, flags?: RenderFlags) { return markdownEngine; } +function reorderEngines(project: ProjectContext) { + // TODO: add `engines` to schema + const userSpecifiedOrder: string[] = + project.config?.engines as string[] | undefined ?? []; + + for (const key of userSpecifiedOrder) { + if (!kEngines.has(key)) { + throw new Error( + `'${key}' was specified in the list of engines in the project settings but it is not a valid engine. Available engines are ${ + Array.from(kEngines.keys()).join(", ") + }`, + ); + } + } + + const reorderedEngines = new Map(); + + // Add keys in the order of userSpecifiedOrder first + for (const key of userSpecifiedOrder) { + reorderedEngines.set(key, kEngines.get(key)!); // Non-null assertion since we verified the keys are in the map + } + + // Add the rest of the keys from the original map + for (const [key, value] of kEngines) { + if (!reorderedEngines.has(key)) { + reorderedEngines.set(key, value); + } + } + + return reorderedEngines; +} + export async function fileExecutionEngine( file: string, flags: RenderFlags | undefined, @@ -158,8 +194,10 @@ export async function fileExecutionEngine( return undefined; } + const reorderedEngines = reorderEngines(project); + // try to find an engine that claims this extension outright - for (const [_, engine] of kEngines) { + for (const [_, engine] of reorderedEngines) { if (engine.claimsFile(file, ext)) { return engine; } @@ -175,6 +213,7 @@ export async function fileExecutionEngine( try { return markdownExecutionEngine( markdown ? markdown.value : Deno.readTextFileSync(file), + reorderedEngines, flags, ); } catch (error) { diff --git a/src/execute/julia.ts b/src/execute/julia.ts index f83241ef5c..8240891568 100644 --- a/src/execute/julia.ts +++ b/src/execute/julia.ts @@ -3,6 +3,7 @@ import { join } from "../deno_ral/path.ts"; import { MappedString, mappedStringFromFile } from "../core/mapped-text.ts"; import { partitionMarkdown } from "../core/pandoc/pandoc-partition.ts"; import { readYamlFromMarkdown } from "../core/yaml.ts"; +import { asMappedString } from "../core/lib/mapped-text.ts"; import { ProjectContext } from "../project/types.ts"; import { DependenciesOptions, @@ -46,6 +47,10 @@ import { executeResultIncludes, } from "./jupyter/jupyter.ts"; import { isWindows } from "../deno_ral/platform.ts"; +import { + isJupyterPercentScript, + markdownFromJupyterPercentScript, +} from "./jupyter/percent.ts"; export interface JuliaExecuteOptions extends ExecuteOptions { julia_cmd: string; @@ -53,6 +58,10 @@ export interface JuliaExecuteOptions extends ExecuteOptions { supervisor_pid?: number; } +function isJuliaPercentScript(file: string) { + return isJupyterPercentScript(file, [".jl"]); +} + export const juliaEngine: ExecutionEngine = { name: kJuliaEngine, @@ -68,12 +77,12 @@ export const juliaEngine: ExecutionEngine = { validExtensions: () => [], - claimsFile: (file: string, ext: string) => false, + claimsFile: (file: string, _ext: string) => { + return isJuliaPercentScript(file); + }, claimsLanguage: (language: string) => { - // we don't claim `julia` so the old behavior of using the jupyter - // backend by default stays intact - return false; // language.toLowerCase() === "julia"; + return language.toLowerCase() === "julia"; }, partitionedMarkdown: async (file: string) => { @@ -109,7 +118,13 @@ export const juliaEngine: ExecutionEngine = { }, markdownForFile(file: string): Promise { - return Promise.resolve(mappedStringFromFile(file)); + if (isJupyterPercentScript(file)) { + return Promise.resolve( + asMappedString(markdownFromJupyterPercentScript(file)), + ); + } else { + return Promise.resolve(mappedStringFromFile(file)); + } }, execute: async (options: ExecuteOptions): Promise => { diff --git a/src/execute/jupyter/jupyter.ts b/src/execute/jupyter/jupyter.ts index 5bc03c342e..c2846b7e9d 100644 --- a/src/execute/jupyter/jupyter.ts +++ b/src/execute/jupyter/jupyter.ts @@ -150,8 +150,10 @@ export const jupyterEngine: ExecutionEngine = { isJupyterPercentScript(file); }, - claimsLanguage: (_language: string) => { - return false; + claimsLanguage: (language: string) => { + // jupyter has to claim julia so that julia may also claim it without changing the old behavior + // of preferring jupyter over julia engine by default + return language.toLowerCase() === "julia"; }, markdownForFile(file: string): Promise { diff --git a/src/execute/jupyter/percent.ts b/src/execute/jupyter/percent.ts index de3f302626..d73e8d2a30 100644 --- a/src/execute/jupyter/percent.ts +++ b/src/execute/jupyter/percent.ts @@ -20,9 +20,10 @@ export const kJupyterPercentScriptExtensions = [ ".r", ]; -export function isJupyterPercentScript(file: string) { +export function isJupyterPercentScript(file: string, extensions?: string[]) { const ext = extname(file).toLowerCase(); - if (kJupyterPercentScriptExtensions.includes(ext)) { + const availableExtensions = extensions ?? kJupyterPercentScriptExtensions; + if (availableExtensions.includes(ext)) { const text = Deno.readTextFileSync(file); return !!text.match(/^\s*#\s*%%+\s+\[markdown|raw\]/); } else { @@ -77,10 +78,10 @@ export function markdownFromJupyterPercentScript(file: string) { let rawContent = cellContent(cellLines); const format = cell.header?.metadata?.["format"]; const mimeType = cell.header.metadata?.[kCellRawMimeType]; - if (typeof (mimeType) === "string") { + if (typeof mimeType === "string") { const rawBlock = mdRawOutput(mimeType, lines(rawContent)); rawContent = rawBlock || rawContent; - } else if (typeof (format) === "string") { + } else if (typeof format === "string") { rawContent = mdFormatOutput(format, lines(rawContent)); } markdown += rawContent; From 8e761969f58a27d66e96758d73901dc10d1ca61e Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Wed, 8 Jan 2025 08:49:14 +0100 Subject: [PATCH 2/7] add engines to schema --- src/resources/schema/project.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/resources/schema/project.yml b/src/resources/schema/project.yml index c16980d43f..f2348ef330 100644 --- a/src/resources/schema/project.yml +++ b/src/resources/schema/project.yml @@ -84,3 +84,8 @@ # # In general, full json schema would allow negative assertions, # but that makes our error localization heuristics worse. So we hack. + +- name: engines + schema: + arrayOf: string + description: "List execution engines you want to give priority when determining which engine should render a notebook. If two engines have support for a notebook, the one listed earlier will be chosen. Quarto's default order is 'knitr', 'jupyter', 'markdown', 'julia'." From 3e6771227eb6b438e9c35d8a56a8cb9a15640f69 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Wed, 8 Jan 2025 11:20:28 +0100 Subject: [PATCH 3/7] add tests for `engines` key effect --- .../engine-reordering/julia-engine/_quarto.yml | 3 +++ .../engine-reordering/julia-engine/notebook.qmd | 10 ++++++++++ .../engine-reordering/jupyter-engine/_quarto.yml | 3 +++ .../engine-reordering/jupyter-engine/notebook.qmd | 10 ++++++++++ 4 files changed, 26 insertions(+) create mode 100644 tests/docs/smoke-all/engine-reordering/julia-engine/_quarto.yml create mode 100644 tests/docs/smoke-all/engine-reordering/julia-engine/notebook.qmd create mode 100644 tests/docs/smoke-all/engine-reordering/jupyter-engine/_quarto.yml create mode 100644 tests/docs/smoke-all/engine-reordering/jupyter-engine/notebook.qmd diff --git a/tests/docs/smoke-all/engine-reordering/julia-engine/_quarto.yml b/tests/docs/smoke-all/engine-reordering/julia-engine/_quarto.yml new file mode 100644 index 0000000000..3aaa4272ac --- /dev/null +++ b/tests/docs/smoke-all/engine-reordering/julia-engine/_quarto.yml @@ -0,0 +1,3 @@ +project: + type: default +engines: ["julia"] diff --git a/tests/docs/smoke-all/engine-reordering/julia-engine/notebook.qmd b/tests/docs/smoke-all/engine-reordering/julia-engine/notebook.qmd new file mode 100644 index 0000000000..3445e8af0a --- /dev/null +++ b/tests/docs/smoke-all/engine-reordering/julia-engine/notebook.qmd @@ -0,0 +1,10 @@ +```{julia} +using Test +@test haskey( + Base.loaded_modules, + Base.PkgId( + Base.UUID("38328d9c-a911-4051-bc06-3f7f556ffeda"), + "QuartoNotebookWorker", + ) +) +``` \ No newline at end of file diff --git a/tests/docs/smoke-all/engine-reordering/jupyter-engine/_quarto.yml b/tests/docs/smoke-all/engine-reordering/jupyter-engine/_quarto.yml new file mode 100644 index 0000000000..548cad7ecd --- /dev/null +++ b/tests/docs/smoke-all/engine-reordering/jupyter-engine/_quarto.yml @@ -0,0 +1,3 @@ +project: + type: default +engines: ["jupyter"] diff --git a/tests/docs/smoke-all/engine-reordering/jupyter-engine/notebook.qmd b/tests/docs/smoke-all/engine-reordering/jupyter-engine/notebook.qmd new file mode 100644 index 0000000000..24ff8511a0 --- /dev/null +++ b/tests/docs/smoke-all/engine-reordering/jupyter-engine/notebook.qmd @@ -0,0 +1,10 @@ +```{julia} +using Test +@test haskey( + Base.loaded_modules, + Base.PkgId( + Base.UUID("7073ff75-c697-5162-941a-fcdaad2a7d2a"), + "IJulia", + ) +) +``` \ No newline at end of file From d37188fe70478899214cd8a5766e1502a5bd3de8 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Wed, 8 Jan 2025 11:52:42 +0100 Subject: [PATCH 4/7] add test for invalid engine --- tests/docs/engine/invalid-project/_quarto.yml | 1 + .../docs/engine/invalid-project/notebook.qmd | 0 .../engine/invalid-engine-in-project.test.ts | 19 +++++++++++++++++++ 3 files changed, 20 insertions(+) create mode 100644 tests/docs/engine/invalid-project/_quarto.yml create mode 100644 tests/docs/engine/invalid-project/notebook.qmd create mode 100644 tests/smoke/engine/invalid-engine-in-project.test.ts diff --git a/tests/docs/engine/invalid-project/_quarto.yml b/tests/docs/engine/invalid-project/_quarto.yml new file mode 100644 index 0000000000..70cc2f16e0 --- /dev/null +++ b/tests/docs/engine/invalid-project/_quarto.yml @@ -0,0 +1 @@ +engines: ["invalid-engine"] diff --git a/tests/docs/engine/invalid-project/notebook.qmd b/tests/docs/engine/invalid-project/notebook.qmd new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/smoke/engine/invalid-engine-in-project.test.ts b/tests/smoke/engine/invalid-engine-in-project.test.ts new file mode 100644 index 0000000000..c40dcf125d --- /dev/null +++ b/tests/smoke/engine/invalid-engine-in-project.test.ts @@ -0,0 +1,19 @@ +import { assertRejects } from "https://deno.land/std/assert/assert_rejects.ts"; +import { quarto } from "../../../src/quarto.ts"; +import { test } from "../../test.ts"; + +test( + { + name: "invalid engines option errors", + execute: async () => { + assertRejects( + async () => {await quarto(["render", "docs/engine/invalid-project/notebook.qmd"])}, + Error, + "'invalid-engine' was specified in the list of engines in the project settings but it is not a valid engine", + ) + }, + type: "smoke", + context: {}, + verify: [], + } +) \ No newline at end of file From f4bc9a7b6fa5d7b8f7dd9ee13079e9dfdd084d09 Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Wed, 8 Jan 2025 12:20:51 +0100 Subject: [PATCH 5/7] use testing/asserts import path --- tests/smoke/engine/invalid-engine-in-project.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/smoke/engine/invalid-engine-in-project.test.ts b/tests/smoke/engine/invalid-engine-in-project.test.ts index c40dcf125d..4b7081e94f 100644 --- a/tests/smoke/engine/invalid-engine-in-project.test.ts +++ b/tests/smoke/engine/invalid-engine-in-project.test.ts @@ -1,4 +1,4 @@ -import { assertRejects } from "https://deno.land/std/assert/assert_rejects.ts"; +import { assertRejects } from "testing/asserts"; import { quarto } from "../../../src/quarto.ts"; import { test } from "../../test.ts"; From 419c313f3dea9988ba5691fb674c5d5e87351fad Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Tue, 14 Jan 2025 18:22:34 +0100 Subject: [PATCH 6/7] remove stale todo --- src/execute/engine.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/execute/engine.ts b/src/execute/engine.ts index 1c869a6f6b..5a1f7e886b 100644 --- a/src/execute/engine.ts +++ b/src/execute/engine.ts @@ -148,7 +148,6 @@ export function markdownExecutionEngine( } function reorderEngines(project: ProjectContext) { - // TODO: add `engines` to schema const userSpecifiedOrder: string[] = project.config?.engines as string[] | undefined ?? []; From 18aefad4e8873fb3d7fa921ec9d794185cb196ca Mon Sep 17 00:00:00 2001 From: Julius Krumbiegel Date: Tue, 14 Jan 2025 18:22:52 +0100 Subject: [PATCH 7/7] change `isJupyterPercentScript` to `isJuliaPercentScript` --- src/execute/julia.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/execute/julia.ts b/src/execute/julia.ts index 8240891568..ea7ca8903b 100644 --- a/src/execute/julia.ts +++ b/src/execute/julia.ts @@ -118,7 +118,7 @@ export const juliaEngine: ExecutionEngine = { }, markdownForFile(file: string): Promise { - if (isJupyterPercentScript(file)) { + if (isJuliaPercentScript(file)) { return Promise.resolve( asMappedString(markdownFromJupyterPercentScript(file)), );