Removing the notebookDeprecated proposal

[mjbvz]

The notebookDeprecated api proposal is currently not on track for finalization. It only contains a single property NotebookCellOutput.id, which we believe is no longer useful or needed

We plan on removing this proposal it once all current consumers have migrated off it. Here's the current list:

• Jupyter @rchiodo @DonJayamanne
• Data Wrangler @pwang347
• Dot Net Interactive @brettfo, @colombod

Please check if your extension is using this api and if you have any concerns migrating off of it

[rchiodo]

Is there an alternative? We are actually using this.
https://github.com/microsoft/vscode-jupyter/blob/bf9b966c06a976c4bfdefebc0eee4012da76bf82/src/notebooks/outputs/plotSaveHandler.node.ts#L101
and here
https://github.com/microsoft/vscode-jupyter/blob/bf9b966c06a976c4bfdefebc0eee4012da76bf82/src/notebooks/outputs/plotViewHandler.node.ts#L47

[mjbvz]

@rebornix and @jrieken can probably clarify this better, but I believe you have a few options depending on your needs:

• Compare NotebookCellOutput objects directly using equality checks.
• Use the index of the output instead of id
• Add your own id into the metadata

[DonJayamanne]

Basically the requirement here is, we need the ability to map the webview OutputItem to an extension host NotebookCellOutput
vscode-notebook-renderer {OutputItem} -> vscode {NotebookCellOutput}

We can add custom metadata, however now we need to ensure the serializer is updated to ignore such custom metadata (but they both live in two separate repos).

Compare NotebookCellOutput objects directly using equality checks.

This isn't possible, on the renderer (webview) side the object is not the same as the object on the extension host.

Use the index of the output instead of

There's no such thing in the webview OutputItem type.

[jrieken]

We can add custom metadata, however now we need to ensure the serializer is updated to ignore such custom metadata

👍 Yeah, way to go. The id seems like specific metadata needed by jupyter only and nothing that other extensions have asked for

(but they both live in two separate repos).

Do you mean that the serializer lives in one repo and the controller/kernel in another? Can there be some "contract" that metadata properties of certain prefix aren't ever saved?

[DonJayamanne]

Do you mean that the serializer lives in one repo and the controller/kernel in another?

Yes

Can there be some "contract" that metadata properties of certain prefix aren't ever saved?

Yes we could, was planning on avoiding this, as the contract is very loose here, basically the code will be if we have metadata named xyz, then don't save it.
Need to see how this impacts SCM (we have changes in model but not serialized, hence not sure what happens when we undo-deserialize).

@mjbvz Would prefer if this API is not dropped just yet, let me check this and get back.

[mjbvz]

@DonJayamanne Yes we don't need to rush removing this but I do want to make sure it happens at some point since this API proposal has already been lingering around for a while. I've opened microsoft/vscode-jupyter#9629 to track this debt work

[rchiodo]

I think the assumption that we don't want metadata in the output could be challenged. We already put metadata in the kernel info. Adding it to the output doesn't seem bad to me.

And writing metadata need only happen when we update an output. Output without metadata.ids could just skip supporting the plot viewer.

Additionally we might not even need it. You could hash the contents of the image and when clicking the plot viewer or plot save button, pass the hash of the image. The extension side code could just look for this image in the output data.

[jrieken]

• id is useful to find ext host output from renderer (command, actions, etc)
• tho id is random and doesn't fit into model as nicely
• think about exposing more information about cell output to renderer like index of the output, index of cell, uri and version of document

[rebornix]

@pwang347 mentioned that this API might still be used by Data Wrangler, we want to make sure that they finish the migration before we remove it.

[jrieken]

@rebornix According to our product.json-based allow listing it isn't. Is data wrangler an extension or are they a fork? In the latter case we should be fine

[sadasant]

@jrieken It’s an extension.

(Good day, everyone.)

[jrieken]

Thanks @sadasant - all should be good because it isn't allow-listed and therefore cannot use the API anyways

[pwang347]

@jrieken we do have this code snippet, which seems to be working

I'm not familiar with the allow-list, but I recall chatting with @DonJayamanne previously about gaining access to the notebook proposed APIs. Not sure if the permissions are set at a different place/granularity. Thanks!

[DonJayamanne]

previously about gaining access to the notebook proposed APIs. Not sure if the permissions are set at a different place/granularity. Thanks!

Yes, I thought I had addressed those by providing the necessary information.
You need to get access to each API individually, there's no setting to give access to all of the proposed APIs.

[pwang347]

previously about gaining access to the notebook proposed APIs. Not sure if the permissions are set at a different place/granularity. Thanks!

Yes, I thought I had addressed those by providing the necessary information. You need to get access to each API individually, there's no setting to give access to all of the proposed APIs.

You're right, my apologies. I don't believe we've explicitly requested for notebookDeprecated, though it is interesting that we're able to access the property from the event still.

[pwang347]

FYI @rebornix @DonJayamanne @mjbvz it seems like the output ID is now going stale in the latest Insiders version, this is causing the Data Wrangler launch button to not show up sometimes.

I verified that the output ID is being updated correctly in VS Code stable.

If this change was done intentionally, do we have an alternative way to figure out which cell an output renderer belongs to so that we can migrate off this API?

[DonJayamanne]

FYI @rebornix @DonJayamanne @mjbvz it seems like the output ID is now going stale in the latest Insiders version,

NOt sure what you mean by this, please can you file an issue in Jupyter extension with steps to repro that.

[pwang347]

FYI @rebornix @DonJayamanne @mjbvz it seems like the output ID is now going stale in the latest Insiders version,

NOt sure what you mean by this, please can you file an issue in Jupyter extension with steps to repro that.

Strange, I can't seem to reproduce this anymore on Insiders, even on older versions of Jupyter and Python. I meant that the outputId in the vscode.NotebookCell interface seemed to not update on new cell runs.

Multiple people reported seeing the problem as well, I'll check with them to see if it has been resolved. Otherwise will look into creating an issue.

Thanks!