From 9c1b8c7314e6931dbb79858edf4a688cabb287a9 Mon Sep 17 00:00:00 2001 From: Angus Hollands Date: Wed, 28 Aug 2024 19:24:05 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=96=20Make=20`name`=20&=20`display=5Fn?= =?UTF-8?q?ame`=20required=20properties=20for=20`KernelSpec`=20(#1490)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .changeset/little-trains-film.md | 5 +++ packages/myst-execute/src/execute.ts | 26 +++++++++---- .../src/kernelspec/kernelspec.yml | 13 +++++-- .../myst-frontmatter/src/kernelspec/types.ts | 4 +- .../src/kernelspec/validators.ts | 37 +++++++++++++++---- packages/myst-frontmatter/src/page/page.yml | 8 +++- 6 files changed, 72 insertions(+), 21 deletions(-) create mode 100644 .changeset/little-trains-film.md diff --git a/.changeset/little-trains-film.md b/.changeset/little-trains-film.md new file mode 100644 index 000000000..ee3369fa0 --- /dev/null +++ b/.changeset/little-trains-film.md @@ -0,0 +1,5 @@ +--- +"myst-frontmatter": minor +--- + +Make `name`, `display_name` soft-required fields for kernelspec diff --git a/packages/myst-execute/src/execute.ts b/packages/myst-execute/src/execute.ts index 7b8078d7b..d0b66571f 100644 --- a/packages/myst-execute/src/execute.ts +++ b/packages/myst-execute/src/execute.ts @@ -1,6 +1,6 @@ import { select, selectAll } from 'unist-util-select'; import type { Logger } from 'myst-cli-utils'; -import type { PageFrontmatter } from 'myst-frontmatter'; +import type { PageFrontmatter, KernelSpec } from 'myst-frontmatter'; import type { Kernel, KernelMessage, Session, SessionManager } from '@jupyterlab/services'; import type { Code, InlineExpression } from 'myst-spec-ext'; import type { IOutput } from '@jupyterlab/nbformat'; @@ -106,7 +106,7 @@ async function evaluateExpression(kernel: Kernel.IKernelConnection, expr: string * * @param nodes array of executable nodes */ -function buildCacheKey(nodes: (ICellBlock | InlineExpression)[]): string { +function buildCacheKey(kernelSpec: KernelSpec, nodes: (ICellBlock | InlineExpression)[]): string { // Build an array of hashable items from an array of nodes const hashableItems: { kind: string; @@ -129,9 +129,12 @@ function buildCacheKey(nodes: (ICellBlock | InlineExpression)[]): string { }); } } - // Serialize the array into JSON, and compute the hash - const hashableString = JSON.stringify(hashableItems); - return createHash('md5').update(hashableString).digest('hex'); + + // Build a hash from notebook state + return createHash('md5') + .update(kernelSpec.name) + .update(JSON.stringify(hashableItems)) + .digest('hex'); } type ICellBlockOutput = GenericNode & { @@ -281,6 +284,15 @@ export type Options = { */ export async function kernelExecutionTransform(tree: GenericParent, vfile: VFile, opts: Options) { const log = opts.log ?? console; + + // We need the kernelspec to proceed + if (opts.frontmatter.kernelspec === undefined) { + return fileError( + vfile, + `Notebook does not declare the necessary 'kernelspec' frontmatter key required for execution`, + ); + } + // Pull out code-like nodes const executableNodes = selectAll(`block[kind=${NotebookCell.code}],inlineExpression`, tree) as ( | ICellBlock @@ -293,7 +305,7 @@ export async function kernelExecutionTransform(tree: GenericParent, vfile: VFile } // See if we already cached this execution - const cacheKey = buildCacheKey(executableNodes); + const cacheKey = buildCacheKey(opts.frontmatter.kernelspec, executableNodes); let cachedResults: (IExpressionResult | IOutput[])[] | undefined = opts.cache.get(cacheKey); // Do we need to re-execute notebook? @@ -318,7 +330,7 @@ export async function kernelExecutionTransform(tree: GenericParent, vfile: VFile type: 'notebook', name: path.basename(vfile.path), kernel: { - name: opts.frontmatter?.kernelspec?.name ?? 'python3', + name: opts.frontmatter.kernelspec.name, }, }; await sessionManager diff --git a/packages/myst-frontmatter/src/kernelspec/kernelspec.yml b/packages/myst-frontmatter/src/kernelspec/kernelspec.yml index 48f08f6de..aa81f53be 100644 --- a/packages/myst-frontmatter/src/kernelspec/kernelspec.yml +++ b/packages/myst-frontmatter/src/kernelspec/kernelspec.yml @@ -1,16 +1,23 @@ title: Kernel Spec cases: - - title: empty object returns self + - title: empty object has defaults raw: kernelspec: {} normalized: - kernelspec: {} + kernelspec: + name: python3 + display_name: python3 Kernel + warnings: 2 - title: extra keys removed raw: kernelspec: extra: '' + name: python3 + display_name: Python 3 normalized: - kernelspec: {} + kernelspec: + name: python3 + display_name: Python 3 warnings: 1 - title: full object returns self raw: diff --git a/packages/myst-frontmatter/src/kernelspec/types.ts b/packages/myst-frontmatter/src/kernelspec/types.ts index d3fd23666..2927bbe94 100644 --- a/packages/myst-frontmatter/src/kernelspec/types.ts +++ b/packages/myst-frontmatter/src/kernelspec/types.ts @@ -1,7 +1,7 @@ export type KernelSpec = { - name?: string; + name: string; + display_name: string; language?: string; - display_name?: string; argv?: string[]; env?: Record; }; diff --git a/packages/myst-frontmatter/src/kernelspec/validators.ts b/packages/myst-frontmatter/src/kernelspec/validators.ts index a02d638b6..045cdef45 100644 --- a/packages/myst-frontmatter/src/kernelspec/validators.ts +++ b/packages/myst-frontmatter/src/kernelspec/validators.ts @@ -6,10 +6,11 @@ import { validateObject, validateObjectKeys, validateString, + validationWarning, } from 'simple-validators'; import type { KernelSpec } from './types.js'; -const KERNELSPEC_KEYS = ['name', 'language', 'display_name', 'argv', 'env']; +const KERNELSPEC_KEYS = ['name', 'display_name', 'language', 'argv', 'env']; /** * Validate KernelSpec object @@ -17,18 +18,40 @@ const KERNELSPEC_KEYS = ['name', 'language', 'display_name', 'argv', 'env']; export function validateKernelSpec(input: any, opts: ValidationOptions) { const value = validateObjectKeys(input, { optional: KERNELSPEC_KEYS }, opts); if (value === undefined) return undefined; - const output: KernelSpec = {}; + + // name is a required member of KernelSpec, but hasn't previously been required + let name: string; if (defined(value.name)) { - output.name = validateString(value.name, incrementOptions('name', opts)); - } - if (defined(value.language)) { - output.language = validateString(value.language, incrementOptions('language', opts)); + const validatedName = validateString(value.name, incrementOptions('name', opts)); + if (validatedName === undefined) return undefined; + name = validatedName; + } else { + name = 'python3'; + validationWarning(`"name" key is required; using '${name}' as placeholder value`, opts); } + + // The same logic for `display_name` applies. It is usually ignored by frontends + // in favour of the actual kernel information, so we assign an arbitrary value for now. + // Note that jupytext complains if its missing, so we should push people to set it + let displayName: string; if (defined(value.display_name)) { - output.display_name = validateString( + const validatedDisplayName = validateString( value.display_name, incrementOptions('display_name', opts), ); + if (validatedDisplayName === undefined) return undefined; + displayName = validatedDisplayName; + } else { + displayName = `${name} Kernel`; + validationWarning( + `"display_name" key is required; using '${displayName}' as placeholder value`, + opts, + ); + } + + const output: KernelSpec = { name, display_name: displayName }; + if (defined(value.language)) { + output.language = validateString(value.language, incrementOptions('language', opts)); } if (defined(value.env)) { output.env = validateObject(value.env, incrementOptions('env', opts)); diff --git a/packages/myst-frontmatter/src/page/page.yml b/packages/myst-frontmatter/src/page/page.yml index eeeb9f441..9badc2dd6 100644 --- a/packages/myst-frontmatter/src/page/page.yml +++ b/packages/myst-frontmatter/src/page/page.yml @@ -41,7 +41,9 @@ cases: subtitle: sub short_title: short date: '14 Dec 2021' - kernelspec: {} + kernelspec: + name: python3 + display_name: Python 3 jupytext: {} keywords: - example @@ -85,7 +87,9 @@ cases: subtitle: sub short_title: short date: '2021-12-14' - kernelspec: {} + kernelspec: + name: python3 + display_name: Python 3 jupytext: {} keywords: - example