Add cell id per notebook format 4.5 (#10018)

* Add cell id to cell models

* Set the cell id from the cell source by default.

* Delete cell ids if the notebook format is 4.0-4.4.

* wip

* Revert format changes, clarify comment

The existing code will automatically upgrade the notebook format version if it comes through as a newer version, so we just need to keep track of the earliest version we support as the default.

* If the notebook format is 4.5 or greater, use the cell id from the cell.

* Lint

* Add tests for cell ids

* Lint/integrity

Co-authored-by: Jay Qi <jayqi@users.noreply.github.com>
Co-authored-by: Jason Grout <jgrout6@bloomberg.net>

packages/cells/package.json

@@ -39,6 +39,7 @@
     "test:cov": "jest --collect-coverage",
     "test:debug": "node --inspect-brk node_modules/.bin/jest --runInBand",
     "test:debug:watch": "node --inspect-brk node_modules/.bin/jest --runInBand --watch",
+    "test:watch": "jest --runInBand --watch",
     "watch": "tsc -b --watch"
   },
   "dependencies": {

packages/cells/src/model.ts

@@ -164,7 +164,7 @@ export class CellModel extends CodeEditor.Model implements ICellModel {
   constructor(options: CellModel.IOptions) {
     super({ modelDB: options.modelDB });
 
-    this.id = options.id || UUID.uuid4();
+    this.id = options.id || (options.cell?.id as string) || UUID.uuid4();
 
     this.value.changed.connect(this.onGenericChange, this);
 
@@ -424,7 +424,9 @@ export class RawCellModel extends AttachmentsCellModel {
    * Serialize the model to JSON.
    */
   toJSON(): nbformat.IRawCell {
-    return super.toJSON() as nbformat.IRawCell;
+    const cell = super.toJSON() as nbformat.IRawCell;
+    cell.id = this.id;
+    return cell;
   }
 }
 
@@ -452,7 +454,9 @@ export class MarkdownCellModel extends AttachmentsCellModel {
    * Serialize the model to JSON.
    */
   toJSON(): nbformat.IMarkdownCell {
-    return super.toJSON() as nbformat.IMarkdownCell;
+    const cell = super.toJSON() as nbformat.IMarkdownCell;
+    cell.id = this.id;
+    return cell;
   }
 }
 
@@ -565,6 +569,7 @@ export class CodeCellModel extends CellModel implements ICodeCellModel {
     const cell = super.toJSON() as nbformat.ICodeCell;
     cell.execution_count = this.executionCount || null;
     cell.outputs = this.outputs.toJSON();
+    cell.id = this.id;
     return cell;
   }
 

packages/cells/test/model.spec.ts

@@ -48,12 +48,48 @@ describe('cells/model', () => {
         const cell: nbformat.IRawCell = {
           cell_type: 'raw',
           source: ['foo\n', 'bar\n', 'baz'],
-          metadata: { trusted: false }
+          metadata: { trusted: false },
+          id: 'cell_id'
         };
         const model = new CellModel({ cell });
         expect(model).toBeInstanceOf(CellModel);
         expect(model.value.text).toBe((cell.source as string[]).join(''));
       });
+
+      it('should use the id argument', () => {
+        const cell: nbformat.IRawCell = {
+          cell_type: 'raw',
+          source: ['foo\n', 'bar\n', 'baz'],
+          metadata: { trusted: false },
+          id: 'cell_id'
+        };
+        const model = new CellModel({ cell, id: 'my_id' });
+        expect(model).toBeInstanceOf(CellModel);
+        expect(model.id).toBe('my_id');
+      });
+
+      it('should use the cell id if an id is not supplied', () => {
+        const cell: nbformat.IRawCell = {
+          cell_type: 'raw',
+          source: ['foo\n', 'bar\n', 'baz'],
+          metadata: { trusted: false },
+          id: 'cell_id'
+        };
+        const model = new CellModel({ cell });
+        expect(model).toBeInstanceOf(CellModel);
+        expect(model.id).toBe('cell_id');
+      });
+
+      it('should generate an id if an id or cell id is not supplied', () => {
+        const cell = {
+          cell_type: 'raw',
+          source: ['foo\n', 'bar\n', 'baz'],
+          metadata: { trusted: false }
+        };
+        const model = new CellModel({ cell });
+        expect(model).toBeInstanceOf(CellModel);
+        expect(model.id.length).toBeGreaterThan(0);
+      });
     });
 
     describe('#contentChanged', () => {
@@ -243,6 +279,20 @@ describe('cells/model', () => {
         expect(model.type).toBe('raw');
       });
     });
+    describe('#toJSON()', () => {
+      it('should return a raw cell encapsulation of the model value', () => {
+        const cell: nbformat.IRawCell = {
+          cell_type: 'raw',
+          source: 'foo',
+          metadata: {},
+          id: 'cell_id'
+        };
+        const model = new RawCellModel({ cell });
+        const serialized = model.toJSON();
+        expect(serialized).not.toBe(cell);
+        expect(serialized).toEqual(cell);
+      });
+    });
   });
 
   describe('MarkdownCellModel', () => {
@@ -252,6 +302,20 @@ describe('cells/model', () => {
         expect(model.type).toBe('markdown');
       });
     });
+    describe('#toJSON()', () => {
+      it('should return a markdown cell encapsulation of the model value', () => {
+        const cell: nbformat.IMarkdownCell = {
+          cell_type: 'markdown',
+          source: 'foo',
+          metadata: {},
+          id: 'cell_id'
+        };
+        const model = new MarkdownCellModel({ cell });
+        const serialized = model.toJSON();
+        expect(serialized).not.toBe(cell);
+        expect(serialized).toEqual(cell);
+      });
+    });
   });
 
   describe('CodeCellModel', () => {
@@ -433,7 +497,8 @@ describe('cells/model', () => {
             } as nbformat.IDisplayData
           ],
           source: 'foo',
-          metadata: { trusted: false }
+          metadata: { trusted: false },
+          id: 'cell_id'
         };
         const model = new CodeCellModel({ cell });
         const serialized = model.toJSON();

packages/nbformat/src/index.ts

@@ -12,12 +12,12 @@
 import { PartialJSONObject, JSONExt } from '@lumino/coreutils';
 
 /**
- * The major version of the notebook format.
+ * The earliest major version of the notebook format we support.
  */
 export const MAJOR_VERSION: number = 4;
 
 /**
- * The minor version of the notebook format.
+ * The earliest minor version of the notebook format we support.
  */
 export const MINOR_VERSION: number = 4;
 
@@ -218,6 +218,14 @@ export interface IRawCellMetadata extends IBaseCellMetadata {
  * A raw cell.
  */
 export interface IRawCell extends IBaseCell {
+  /**
+   * A string field representing the identifier of this particular cell.
+   *
+   * Notebook format 4.4 requires no id field, but format 4.5 requires an id
+   * field. We need to handle both cases, so we make id optional here.
+   */
+  id?: string;
+
   /**
    * String identifying the type of cell.
    */
@@ -238,6 +246,14 @@ export interface IRawCell extends IBaseCell {
  * A markdown cell.
  */
 export interface IMarkdownCell extends IBaseCell {
+  /**
+   * A string field representing the identifier of this particular cell.
+   *
+   * Notebook format 4.4 requires no id field, but format 4.5 requires an id
+   * field. We need to handle both cases, so we make id optional here.
+   */
+  id?: string;
+
   /**
    * String identifying the type of cell.
    */
@@ -283,6 +299,14 @@ export interface ICodeCellMetadata extends IBaseCellMetadata {
  * A code cell.
  */
 export interface ICodeCell extends IBaseCell {
+  /**
+   * A string field representing the identifier of this particular cell.
+   *
+   * Notebook format 4.4 requires no id field, but format 4.5 requires an id
+   * field. We need to handle both cases, so we make id optional here.
+   */
+  id?: string;
+
   /**
    * String identifying the type of cell.
    */

packages/notebook/package.json

@@ -38,6 +38,7 @@
     "test:cov": "jest --collect-coverage",
     "test:debug": "node --inspect-brk node_modules/.bin/jest --runInBand",
     "test:debug:watch": "node --inspect-brk node_modules/.bin/jest --runInBand --watch",
+    "test:watch": "jest --runInBand --watch",
     "watch": "tsc -b --watch"
   },
   "dependencies": {

packages/notebook/src/model.ts

@@ -191,9 +191,13 @@ export class NotebookModel extends DocumentModel implements INotebookModel {
    */
   toJSON(): nbformat.INotebookContent {
     const cells: nbformat.ICell[] = [];
-    for (let i = 0; i < (this.cells?.length || 0); i++) {
-      const cell = this.cells.get(i);
-      cells.push(cell.toJSON());
+    for (let i = 0; i < (this.cells?.length ?? 0); i++) {
+      const cell = this.cells.get(i).toJSON();
+      if (this._nbformat === 4 && this._nbformatMinor <= 4) {
+        // strip cell ids if we have notebook format 4.0-4.4
+        delete cell.id;
+      }
+      cells.push(cell);
     }
     this._ensureMetadata();
     const metadata = Object.create(null) as nbformat.INotebookMetadata;
@@ -217,16 +221,21 @@ export class NotebookModel extends DocumentModel implements INotebookModel {
   fromJSON(value: nbformat.INotebookContent): void {
     const cells: ICellModel[] = [];
     const factory = this.contentFactory;
+    const useId = value.nbformat === 4 && value.nbformat_minor >= 5;
     for (const cell of value.cells) {
+      const options: CellModel.IOptions = { cell };
+      if (useId) {
+        options.id = (cell as any).id;
+      }
       switch (cell.cell_type) {
         case 'code':
-          cells.push(factory.createCodeCell({ cell }));
+          cells.push(factory.createCodeCell(options));
           break;
         case 'markdown':
-          cells.push(factory.createMarkdownCell({ cell }));
+          cells.push(factory.createMarkdownCell(options));
           break;
         case 'raw':
-          cells.push(factory.createRawCell({ cell }));
+          cells.push(factory.createRawCell(options));
           break;
         default:
           continue;

packages/notebook/src/widget.ts

@@ -386,11 +386,14 @@ export class StaticNotebook extends Widget {
     }
     this._updateMimetype();
     const cells = newValue.cells;
+
+    // If there are no cells, create an empty cell
     if (!cells.length) {
       cells.push(
         newValue.contentFactory.createCell(this.notebookConfig.defaultCell, {})
       );
     }
+
     each(cells, (cell: ICellModel, i: number) => {
       this._insertCell(i, cell);
     });

packages/notebook/test/actions.spec.ts

@@ -650,7 +650,7 @@ describe('@jupyterlab/notebook', () => {
       });
 
       it('should clear the existing selection', async () => {
-        const next = widget.widgets[2];
+        const next = widget.widgets[3];
         widget.select(next);
         const result = await NotebookActions.runAndAdvance(
           widget,

packages/notebook/test/model.spec.ts

@@ -74,7 +74,7 @@ describe('@jupyterlab/notebook', () => {
         model.cells.push(cell);
         model.fromJSON(utils.DEFAULT_CONTENT);
         expect(ArrayExt.firstIndexOf(toArray(model.cells), cell)).toBe(-1);
-        expect(model.cells.length).toBe(6);
+        expect(model.cells.length).toBe(7);
       });
 
       it('should allow undoing a change', () => {
@@ -259,7 +259,7 @@ describe('@jupyterlab/notebook', () => {
         model.fromJSON(utils.DEFAULT_CONTENT);
         const text = model.toString();
         const data = JSON.parse(text);
-        expect(data.cells.length).toBe(6);
+        expect(data.cells.length).toBe(7);
       });
     });
 
@@ -267,7 +267,7 @@ describe('@jupyterlab/notebook', () => {
       it('should deserialize the model from a string', () => {
         const model = new NotebookModel();
         model.fromString(JSON.stringify(utils.DEFAULT_CONTENT));
-        expect(model.cells.length).toBe(6);
+        expect(model.cells.length).toBe(7);
       });
 
       it('should set the dirty flag', () => {
@@ -283,19 +283,45 @@ describe('@jupyterlab/notebook', () => {
         const model = new NotebookModel();
         model.fromJSON(utils.DEFAULT_CONTENT);
         const data = model.toJSON();
-        expect(data.cells.length).toBe(6);
+        expect(data.cells.length).toBe(7);
+      });
+      it('should serialize format 4.4 or earlier without cell ids', () => {
+        const model = new NotebookModel();
+        model.fromJSON(utils.DEFAULT_CONTENT);
+        const data = model.toJSON();
+        expect(data.nbformat).toBe(4);
+        expect(data.nbformat_minor).toBeLessThanOrEqual(4);
+        expect(data.cells.length).toBe(7);
+        expect(data.cells[0].id).toBeUndefined();
+      });
+      it('should serialize format 4.5 or later with cell ids', () => {
+        const model = new NotebookModel();
+        model.fromJSON(utils.DEFAULT_CONTENT_45);
+        const data = model.toJSON();
+        expect(data.cells.length).toBe(7);
+        expect(data.cells[0].id).toBe('cell_1');
       });
     });
 
     describe('#fromJSON()', () => {
-      it('should serialize the model from JSON', () => {
+      it('should serialize the model from format<=4.4 JSON', () => {
         const model = new NotebookModel();
         model.fromJSON(utils.DEFAULT_CONTENT);
-        expect(model.cells.length).toBe(6);
+        expect(model.cells.length).toBe(7);
         expect(model.nbformat).toBe(utils.DEFAULT_CONTENT.nbformat);
         expect(model.nbformatMinor).toBe(nbformat.MINOR_VERSION);
       });
 
+      it('should serialize the model from format 4.5 JSON', () => {
+        const model = new NotebookModel();
+        const json = utils.DEFAULT_CONTENT_45;
+        model.fromJSON(json);
+        expect(model.cells.length).toBe(7);
+        expect(model.nbformat).toBe(json.nbformat);
+        expect(model.nbformatMinor).toBe(json.nbformat_minor);
+        expect(model.cells.get(0).id).toBe('cell_1');
+      });
+
       it('should set the dirty flag', () => {
         const model = new NotebookModel();
         model.dirty = false;

packages/notebook/test/utils.ts

@@ -69,6 +69,7 @@ export function populateNotebook(notebook: Notebook): void {
 }
 
 export const DEFAULT_CONTENT = NBTestUtils.DEFAULT_CONTENT;
+export const DEFAULT_CONTENT_45 = NBTestUtils.DEFAULT_CONTENT_45;
 export const editorFactory = NBTestUtils.editorFactory;
 export const mimeTypeService = NBTestUtils.mimeTypeService;
 export const defaultEditorConfig = NBTestUtils.defaultEditorConfig;

packages/notebook/test/widget.spec.ts

@@ -262,7 +262,7 @@ describe('@jupyter/notebook', () => {
         const model = new NotebookModel();
         model.fromJSON(utils.DEFAULT_CONTENT);
         widget.model = model;
-        expect(widget.widgets.length).toBe(6);
+        expect(widget.widgets.length).toBe(model.cells.length);
       });
 
       it('should add a default cell if the notebook model is empty', () => {
@@ -324,7 +324,7 @@ describe('@jupyter/notebook', () => {
         it('should handle an add', () => {
           const cell = widget.model!.contentFactory.createCodeCell({});
           widget.model!.cells.push(cell);
-          expect(widget.widgets.length).toBe(7);
+          expect(widget.widgets.length).toBe(widget.model!.cells.length);
           const child = widget.widgets[0];
           expect(child.hasClass('jp-Notebook-cell')).toBe(true);
         });
@@ -335,9 +335,13 @@ describe('@jupyter/notebook', () => {
           cell1.value.text = '# Hello';
           widget.model!.cells.push(cell1);
           widget.model!.cells.push(cell2);
-          expect(widget.widgets.length).toBe(8);
-          const child1 = widget.widgets[6] as MarkdownCell;
-          const child2 = widget.widgets[7] as MarkdownCell;
+          expect(widget.widgets.length).toBe(widget.model!.cells.length);
+          const child1 = widget.widgets[
+            widget.model!.cells.length - 2
+          ] as MarkdownCell;
+          const child2 = widget.widgets[
+            widget.model!.cells.length - 1
+          ] as MarkdownCell;
           expect(child1.rendered).toBe(true);
           expect(child2.rendered).toBe(false);
         });
@@ -447,7 +451,7 @@ describe('@jupyter/notebook', () => {
         const widget = createWidget();
         expect(widget.widgets.length).toBe(1);
         widget.model!.fromJSON(utils.DEFAULT_CONTENT);
-        expect(widget.widgets.length).toBe(6);
+        expect(widget.widgets.length).toBe(utils.DEFAULT_CONTENT.cells.length);
       });
     });
 
@@ -752,7 +756,7 @@ describe('@jupyter/notebook', () => {
         widget.activeCellIndex = -2;
         expect(widget.activeCellIndex).toBe(0);
         widget.activeCellIndex = 100;
-        expect(widget.activeCellIndex).toBe(5);
+        expect(widget.activeCellIndex).toBe(widget.model!.cells.length - 1);
       });
 
       it('should emit the `stateChanged` signal', () => {
@@ -1218,7 +1222,8 @@ describe('@jupyter/notebook', () => {
         });
 
         it('should not extend a selection if there is text selected in the output', () => {
-          widget.activeCellIndex = 2;
+          const codeCellIndex = 3;
+          widget.activeCellIndex = codeCellIndex;
 
           // Set a selection in the active cell outputs.
           const selection = window.getSelection()!;
@@ -1227,8 +1232,10 @@ describe('@jupyter/notebook', () => {
           );
 
           // Shift click below, which should not extend cells selection.
-          simulate(widget.widgets[4].node, 'mousedown', { shiftKey: true });
-          expect(widget.activeCellIndex).toBe(2);
+          simulate(widget.widgets[codeCellIndex + 2].node, 'mousedown', {
+            shiftKey: true
+          });
+          expect(widget.activeCellIndex).toBe(codeCellIndex);
           expect(selected(widget)).toEqual([]);
         });
 

testutils/default-45.json

@@ -0,0 +1,168 @@
+{
+  "cells": [
+    {
+      "id": "cell_1",
+      "cell_type": "code",
+      "execution_count": 1,
+      "metadata": {
+        "tags": []
+      },
+      "outputs": [
+        {
+          "name": "stdout",
+          "output_type": "stream",
+          "text": ["hello world\n", "0\n", "1\n", "2\n"]
+        },
+        {
+          "name": "stderr",
+          "output_type": "stream",
+          "text": ["output to stderr\n"]
+        },
+        {
+          "name": "stdout",
+          "output_type": "stream",
+          "text": ["some more stdout text\n"]
+        }
+      ],
+      "source": [
+        "import sys\n",
+        "sys.stdout.write('hello world\\n')\n",
+        "sys.stdout.flush()\n",
+        "for i in range(3):\n",
+        "    sys.stdout.write('%s\\n' % i)\n",
+        "    sys.stdout.flush()\n",
+        "sys.stderr.write('output to stderr\\n')\n",
+        "sys.stderr.flush()\n",
+        "sys.stdout.write('some more stdout text\\n')\n",
+        "sys.stdout.flush()"
+      ]
+    },
+    {
+      "id": "cell_2",
+      "cell_type": "markdown",
+      "metadata": {
+        "tags": []
+      },
+      "source": [
+        "# Markdown Cell\n",
+        "\n",
+        "$ e^{ \\pm i\\theta } = \\cos \\theta \\pm i\\sin \\theta + \\beta $\n",
+        "\n",
+        "*It* **really** is!"
+      ]
+    },
+    {
+      "id": "cell_3",
+      "cell_type": "raw",
+      "metadata": {
+        "tags": []
+      },
+      "source": ["Raw Cell\n", "\n", "Second line"]
+    },
+
+    {
+      "id": "cell_4",
+      "cell_type": "code",
+      "execution_count": 2,
+      "metadata": {
+        "tags": []
+      },
+      "outputs": [
+        {
+          "ename": "SyntaxError",
+          "evalue": "invalid syntax (<ipython-input-2-6c5185427360>, line 1)",
+          "output_type": "error",
+          "traceback": [
+            "\u001b[0;36m  File \u001b[0;32m\"<ipython-input-2-6c5185427360>\"\u001b[0;36m, line \u001b[0;32m1\u001b[0m\n\u001b[0;31m    this is a syntax error\u001b[0m\n\u001b[0m                   ^\u001b[0m\n\u001b[0;31mSyntaxError\u001b[0m\u001b[0;31m:\u001b[0m invalid syntax\n"
+          ]
+        }
+      ],
+      "source": ["this is a syntax error"]
+    },
+    {
+      "id": "cell_5",
+      "cell_type": "code",
+      "execution_count": null,
+      "metadata": {
+        "tags": []
+      },
+      "outputs": [],
+      "source": ["print('test')"]
+    },
+    {
+      "id": "cell_6",
+      "cell_type": "code",
+      "execution_count": 4,
+      "metadata": {
+        "tags": []
+      },
+      "outputs": [
+        {
+          "data": {
+            "text/latex": [
+              "The mass-energy equivalence is described by the famous equation\n",
+              " \n",
+              "$$E=mc^2$$\n",
+              " \n",
+              "discovered in 1905 by Albert Einstein. \n",
+              "In natural units ($c$ = 1), the formula expresses the identity\n",
+              " \n",
+              "\\begin{equation}\n",
+              "E=m\n",
+              "\\end{equation}"
+            ],
+            "text/plain": ["<IPython.core.display.Latex object>"]
+          },
+          "execution_count": 4,
+          "metadata": {},
+          "output_type": "execute_result"
+        }
+      ],
+      "source": [
+        "from IPython.display import Latex\n",
+        "Latex('''The mass-energy equivalence is described by the famous equation\n",
+        " \n",
+        "$$E=mc^2$$\n",
+        " \n",
+        "discovered in 1905 by Albert Einstein. \n",
+        "In natural units ($c$ = 1), the formula expresses the identity\n",
+        " \n",
+        "\\\\begin{equation}\n",
+        "E=m\n",
+        "\\\\end{equation}''')"
+      ]
+    },
+    {
+      "id": "cell_7",
+      "cell_type": "code",
+      "execution_count": null,
+      "metadata": {
+        "collapsed": true
+      },
+      "outputs": [],
+      "source": []
+    }
+  ],
+  "metadata": {
+    "anaconda-cloud": {},
+    "kernelspec": {
+      "display_name": "Python [default]",
+      "language": "python",
+      "name": "python3"
+    },
+    "language_info": {
+      "codemirror_mode": {
+        "name": "ipython",
+        "version": 3
+      },
+      "file_extension": ".py",
+      "mimetype": "text/x-python",
+      "name": "python",
+      "nbconvert_exporter": "python",
+      "pygments_lexer": "ipython3",
+      "version": "3.5.2"
+    }
+  },
+  "nbformat": 4,
+  "nbformat_minor": 5
+}

testutils/default.json

@@ -49,6 +49,13 @@
         "*It* **really** is!"
       ]
     },
+    {
+      "cell_type": "raw",
+      "metadata": {
+        "tags": []
+      },
+      "source": ["Raw Cell\n", "\n", "Second line"]
+    },
     {
       "cell_type": "code",
       "execution_count": 2,

testutils/src/notebook-utils.ts

@@ -75,6 +75,7 @@ export namespace NBTestUtils {
   ];
 
   export const DEFAULT_CONTENT: nbformat.INotebookContent = require('../default.json') as nbformat.INotebookContent;
+  export const DEFAULT_CONTENT_45: nbformat.INotebookContent = require('../default-45.json') as nbformat.INotebookContent;
 
   export const defaultEditorConfig = { ...StaticNotebook.defaultEditorConfig };