Skip to content

Commit

Permalink
🤖 Make name & display_name required properties for KernelSpec (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
agoose77 authored Aug 28, 2024
1 parent 0812009 commit 9c1b8c7
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 21 deletions.
5 changes: 5 additions & 0 deletions .changeset/little-trains-film.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"myst-frontmatter": minor
---

Make `name`, `display_name` soft-required fields for kernelspec
26 changes: 19 additions & 7 deletions packages/myst-execute/src/execute.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
Expand All @@ -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 & {
Expand Down Expand Up @@ -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
Expand All @@ -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?
Expand All @@ -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
Expand Down
13 changes: 10 additions & 3 deletions packages/myst-frontmatter/src/kernelspec/kernelspec.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
4 changes: 2 additions & 2 deletions packages/myst-frontmatter/src/kernelspec/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export type KernelSpec = {
name?: string;
name: string;
display_name: string;
language?: string;
display_name?: string;
argv?: string[];
env?: Record<string, any>;
};
37 changes: 30 additions & 7 deletions packages/myst-frontmatter/src/kernelspec/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,52 @@ 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
*/
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));
Expand Down
8 changes: 6 additions & 2 deletions packages/myst-frontmatter/src/page/page.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 9c1b8c7

Please sign in to comment.