Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DS: MVP using VS Code notebook API (June Insiders-only) #2180

Closed
greazer opened this issue Mar 9, 2020 · 2 comments
Closed

DS: MVP using VS Code notebook API (June Insiders-only) #2180

greazer opened this issue Mar 9, 2020 · 2 comments
Assignees

Comments

@greazer
Copy link
Member

greazer commented Mar 9, 2020

This epic includes all issues that should be solvable when/if VS Code adds native notebook support.

See microsoft/vscode#91987 for details on the VS Code side of the work.

Instructions to try the new Notebook Editor

  • Install latest VS Code Insiders
  • Install Python extension
  • Ensure you opt into using the Python Insiders Release using the command Python: Switch to Insiders Daily Channel from the command palette.
  • Reload VS Code when prompted to do so.
  • Option 1
    • Open a Python file (wait for extension to get activated),
    • Right click on an ipynb file and select the option Open in preview Notebook Editor.
    • This allows you to use both the new and old editor. E.g. if some feature doesn't work in the new Editor, you can close it and open the file using the old editor and perform the necessary functions (e.g. Selecting Kernels, etc)
  • Option 2
    • Always use the new Notebook Editor.
    • Open your user settings file settings.json, and add the following setting
	"python.experiments.optInto": [
		"NativeNotebook - experiment",
	],
* As the notebook is in preview state, most features might not be available or function properly. In this case you'd need to revert changes to `settings.json` to get back to the old editor.
@greazer greazer changed the title DS: Use VS Code notebook API DS: Insiders MVP using VS Code notebook API May 7, 2020
@microsoft microsoft locked and limited conversation to collaborators May 8, 2020
@DonJayamanne
Copy link
Contributor

DonJayamanne commented May 8, 2020

Issues

Outdated/Resolved

**for VSC**

  • We will need to know the theme.
    • Today we add custom background colors to ensure images/widgets render correctly.
    • Else some output will not render correctly against dark background.
    • Proper fix is to use Jupyter styles, but then we need to map each VSC style/class against Jupyter vars (theme).
    • API in insiders.

TODO

  • Use standard VS Code markdown renderer, if there are no latex equations.
    • Check if this is at all possible.
    • Decided not to
  • Review extending VSC markdown renderer to support latex
    • Decided not to

Changes/Discuss

  • Use standard VS Code markdown renderer, if there are no latex equations.
    • Check if this is at all possible.
    • Add extension for latex in mD
  • Split cellOutput.tsx into smaller parts
    • At least to the level where we can give it output and it will render output
    • It should only handle nbformat.IOutput.
    • If we want to display errors or custom messages with links and the like, then its better for Extension to send these as separate output. This way it will work in all scenarios.
    • E.g. if output is truncated, then extension should send links and information in a new output accordingly, rather than expecting UI to add this stuff.
    • This way it will persist
    • We had issues when adding errors from UI side onto cell, doing this would make things lot simpler.
  • Split execution into separate class
    • Execution should update local NotebookModel with the output.
    • As we update local model, the updated cells are pushed up to the UI
    • This way, extension always has model updated (worked in nb, custom, native and VS Code notebooks).
    • Question: How do we handle Interactive Windows.
      • See next suggestion
  • Interactive Windows will have notebook models
    • This way exporting, execution etc will be consistent across all paradigms (nb & interactive)
    • I.e. model in extension and info sent to UI
    • Exporting, saving, etc all happen on extension, rather than UI sending cells
    • Also cleans up some code
      • Today when we execute a cell, we generate code from cell,
      • THen we generate a cell out of raw text
      • When sending results back, we send cells
      • i.e. we convert from cell to text, then text to cell and back again
      • This will be unnecessary with a unified model

@DonJayamanne
Copy link
Contributor

DonJayamanne commented May 14, 2020

Costing

MVP

Breakdown

Area - Output (4.5)

  • React Renderers for Output (non-ipywidgets) (3.5)
    • (0.5) Include necessary CSS (only what we need, not everything).
    • (2) Copy stuff from cellOutput.tsx or pull it out into separate react control (prototype available, I copied it sideways)
    • (0.5) Create index.tsx purely for new Notebook API with global API to be call ed from renderer (prototype available)
    • (0.5) Bundling (prototype available)

Save notebook (untitled notebooks). (0.5)

  • (0.5) Save
    • Prototype available

Miscellaneous (2)

  • (0.5) Messages for interrupting kernels
    • We have messages for restarting kernels in native editor
    • We need same messages (interrupting kernel API already available)
    • Tested/tests
  • (0.5) Error handler for when execution fails
    • this.errorHandler.handleError(exc).ignoreErrors();
    • Add to executionProvider in prototype
  • (1) Disable code lenses & interactive window related stuff in python files when in nb
    • Send line to terminal, etc
    • Code lenses

Notebook handling (3.5)

  • (1) INativeEditor
    • (1) Handle other public methods on API Prototype available
  • (1) INativeEditorProvider
    • Prototype available
  • Service registrations
    • (0.1) No need of autosave
    • (0.1) Intellisense provider (will need to refactor line by line to get that working without intellisense)
    • (0.5) Remove code related to syncing (completely for custom, vsc nb)
  • (0.5) Update notebook model as we execute cells instead of sending to UI
    • Update VS Code Cells and Notebook as we make changes to INotebookModel
    • Prototype available

Telemetry/Experiments/Keyboard shortcuts & commands (6.5)

  • (1.5) Like for like telemetry
    • Identify all telemetry individually & ensure we have all of them in new Notebook API
  • (2) New experiments library
  • (2) Support Jupyter keyboard shortcuts
    • Prototype available
    • Identify all missing shortcuts and add them
  • (1) Update VS Code contexts
    • Context keys are used to enable/disable commands (show/hide/enable/disable).
    • Tests/Tested

Language features (2)

  • (1.5) Intellisense
    • (1) With Jedi (it works today with our nb, hence should be possible)
      • Prototype available
    • (0.5) Default to MS LSP if Jedi will not work
  • (0.2) Disable refactor, formatting, sorting imports, linting

Non-MVP (47)

Click for breakdown

Area - Output (1.5)

  • React Renderers for Output (non-ipywidgets) (1.5)
    • (0.5) Handle special click events (links, etc) & send a message (postMessage)
      • Generic link click handler for output.
      • prototype available
  • Renderer extension code (1)
    • (0.2) Metadata in images (custom height, backgrounds, retina) (prototype available)
    • (0.2) Adding white/black backgrounds for images when necessary
      • Prototype available
    • (0.5) Display icons to open plots in our (this should be done in extension side of Renderer, leaving React to do absolutely nothing but render)
      • Link in HTML can contain acquireVsCodeAPI().postMessage({type:'openPlot', payload:{cellIdOrPlotId:'xxx'})
      • This way renderer doesn't need to know anything.
      • Prototype available
    • (0) Scrolling of output and the like
      • Discuss: We can control what gets displayed, when to dislplay scroll bars, etc by altering the output (nbformat.CellOutput).
      • I.e. keep logic for scrolling & localized messages in extension
      • Personally I think its best to re-visit this whole scrolling thing.
      • We probably shouldn't do anything as VSC is taking this up Notebook: max output height vscode#93541

IPyWidgets (8)

  • Renderers for IPyWidgets (3)
    • Prototype available
    • (2) Copy stuff from container.tsx and hook up comms
    • (1) Create index.tsx purely widgets
  • Extension Listeners (3)
    • (2) IPyWidgetHandler
    • (1) IPyWidgetScriptSource
  • Refactor our UI tests (2)
    • Launch HTML page act like NB that only displays output

Variable Explorer (9)

  • (1) Jupyter side in Activity Bar
  • (5) Varialbles
    • Variables in a new panel (in Jupyter sidebar panel)
    • Use treeview.
  • (2) Kernels
  • (1) Metadata

Liveshare

  • Won't be doing anything special at this stage.

Save/SaveAs/Rename notebook (untitled notebooks). (2.5)

  • (2) Transition kernel session to right Uri (when saving, save as and rename)
    • Prototype available
  • (0.5) Dispose kernel when closing notebooks
    • Filed issue on VSC (need to know when all editors for file are closed)
    • Prototype available
  • Kernel is no longer attached to nb when using save as when multiple editors are opened
    • Mention to team
      Prototype available

Miscellaneous (4.5)

  • (1) Export/import (ipynb to python, python to ipynb)
    • Move into common area
      in new implementation
  • (0.5) Messages for restarting kernels
    • Assuming its similar to interrupting kernels
  • (3) Kernel selection & Updating metadata
    • Move or copy (little code), but needs to be done in right place
    • Automatically & manually selecting a kernel & updating metadata
    • Move or copy code, but needs to be done in right place
    • Preferably move (to ensure we do things right, as this has a lot of business rules)

Notebook handling (5)

  • (2.5) INativeEditor
    • (2) Pass messages to all the listeners & handle output from listeners
    • (0.5) Handle messages from renderers and pass onto listeners
  • (0.5) Standardize names for notebook objects (we will have 4)
    • nbformat.xx for Jupyter Cells and Output
    • INotebookModel & ICell for out models
    • NotebookData & NotebookCellData for VSCode model
    • NotebookDocument & NotebookCell for VS Code models
    • Refactor to use new naming convention (to discuss if this is confusing). With the new VSC API, this is very confusing.

Toolsbars and runby line (12)

  • (10) Run line by line
  • (2) Toolbars
    • Icons should fire commands
    • Modify existing command handler to delegate the actions.

Language features (4)

  • (2) Hover provider (new work done as part of Run by line)
  • (2) Formatting
    • We'll need to update core extension to handle formatting of non-file based text documents.
    • Suggestion - Support most popular formatters (check telemetry)
  • (2) Formatting
    • Same as formatting

@microsoft microsoft unlocked this conversation May 29, 2020
rchiodo referenced this issue in microsoft/vscode-python May 29, 2020
Note: It is disabled to ensure we do not ship/bundle these things with official VS Code release (not yet).

For #10496

Add notebook renderers
Please note there's a big hack in this code (its to get around a VS Code issue).
Apart from that the rest should be ok
DonJayamanne referenced this issue in microsoft/vscode-python Jun 1, 2020
For #10496

Update our model when user edits VS Code cells
DonJayamanne referenced this issue in microsoft/vscode-python Jun 1, 2020
DonJayamanne referenced this issue in microsoft/vscode-python Jun 2, 2020
DonJayamanne referenced this issue in microsoft/vscode-python Jun 2, 2020
For #10496 #10744

Use vscode.openWith when opening a notebook.
DonJayamanne referenced this issue in microsoft/vscode-python Jun 2, 2020
For #10496

Runing entire notebook and clicking cancel would result in kernel being interrupted n times (where n = number of cells).
I.e. user clicks cancel once, but we end up interrupting multiple times.
DonJayamanne referenced this issue in microsoft/vscode-python Jun 3, 2020
For #10496

Display success when successfully executed, error when there's an error else idle.
DonJayamanne referenced this issue in microsoft/vscode-python Jun 4, 2020
For #10496

* Persist execution times in cell metadata
* Update status if there are errors to include error message

Tests when opening an existing notebook

* Validate metaata
* Validate status message
DonJayamanne referenced this issue in microsoft/vscode-python Jun 8, 2020
For #10496
Use a less hacky way to get the path to the scripts to support split bundles.
DonJayamanne referenced this issue in microsoft/vscode-python Jun 9, 2020
For #10496

Added tests, to ensure changes to API will not break our code (the API used to handle/detect cell manipulations have changed a couple of times)
@greazer greazer changed the title DS: Insiders MVP using VS Code notebook API DS: June (Insiders) MVP using VS Code notebook API Jun 9, 2020
@greazer greazer changed the title DS: June (Insiders) MVP using VS Code notebook API DS: June (Insiders-only) MVP using VS Code notebook API Jun 9, 2020
@greazer greazer changed the title DS: June (Insiders-only) MVP using VS Code notebook API DS: MVP using VS Code notebook API (June Insiders-only) Jun 9, 2020
DonJayamanne referenced this issue in microsoft/vscode-python Jun 12, 2020
For #10496
For #12274

Fixing #12274 only for native notebooks here. Was an easy fix.
DonJayamanne referenced this issue in microsoft/vscode-python Jun 13, 2020
For #10496

Jupyter keyboard shorcuts for cut, copy, paste & split cells
DonJayamanne referenced this issue in microsoft/vscode-python Jun 15, 2020
For #10496

Removed hardcode language to Python (derive from nb metadata if available)
Removed some code/comments related to upstream VSC issues.
DonJayamanne referenced this issue in microsoft/vscode-python Jun 15, 2020
For #10496

Ensure we do not treat notebooks such as Github Issues NB as one of ours.
DonJayamanne referenced this issue in microsoft/vscode-python Jun 16, 2020
For #12376
For #10496

Changes due to the way VSC works.

After VSC calls the backup method, the previous backup is deleted.
This means each backup is unique (hence the backup id must be unique)
Fix for VSC Notebooks and Custom Editor
DonJayamanne referenced this issue in microsoft/vscode-python Jun 18, 2020
For #10496

We don't need the fully qualified path for untitled notebooks.
However auto saving in untitiled nb doesn't work, VSC Master has a fix but we're waiting for insiders to get updated.
DonJayamanne referenced this issue in microsoft/vscode-python Jun 29, 2020
For #10496
Part 2 of #12604
Move code related to updates to INotebookModel into the corresponding INotebookModel class.
DonJayamanne referenced this issue in microsoft/vscode-python Jun 30, 2020
For #10496

Not sure how, but some changes seem to have gone amiss.
DonJayamanne referenced this issue in microsoft/vscode-python Jun 30, 2020
DonJayamanne referenced this issue in microsoft/vscode-python Jun 30, 2020
For #10496

If the API changes, and user is not using the VSC Notebooks or insiders, then swallow errors.
Do not register API against stable version of VSCode.
Also fixed a test.

Basically we need to ensure VSC works when using this API and not crash if the API changes.
DonJayamanne referenced this issue in microsoft/vscode-python Jul 1, 2020
For #10496

Existing icon used for restart icon
@greazer greazer closed this as completed Jul 13, 2020
@DonJayamanne DonJayamanne transferred this issue from microsoft/vscode-python Nov 13, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants