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

Allow the ability to store extension specific metadata in Notebook cells #96433

Closed
TylerLeonhardt opened this issue Apr 28, 2020 · 8 comments
Closed
Assignees
Labels
api feature-request Request for new features or functionality notebook
Milestone

Comments

@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Apr 28, 2020

For the PowerShell extension's Notebook Mode:

The idea is: "There are two types of comment blocks: <# #> (aka block comments) and # (aka line comments). While parsing a script, I want to store metadata in the cell object to say "this cell is backed by block comments not regular comments" so that when I save, the comment type can be honored."

To get around this, I am using a Map<number, CommentType (enum)> in the extension to store this metadata:
https://github.com/TylerLeonhardt/vscode-powershell/blob/fe8576bda498b81f89f05806b2667d8c50e0f6a9/src/features/PowerShellNotebooks.ts#L16

But if I could store this on the cell directly, it would be incredibly safer.

For a Jupyter extension

.ipynb files have a metadata section:
https://gist.github.com/TylerLeonhardt/b51583af068b62be2ca2305aa4061f89?short_path=03c798a#file-azure-powershell-visualization-demo-ipynb-L5-L8

for cell specific details. If this feature was added to Notebook UI in VS Code, I could see a Jupyter extension taking advantage of it as well.

@TylerLeonhardt
Copy link
Member Author

cc @rebornix

@rebornix
Copy link
Member

Pushed first attempt for custom metadata to master, the metadata interface now looks like

export interface NotebookCellMetadata {
    editable?: boolean;
    runnable?: boolean;
    executionOrder?: number;
    statusMessage?: string;
    runState?: NotebookCellRunState;
    runStartTime?: number;
    lastRunDuration?: number;

    /**
     * Additional attributes of a cell metadata.
     */
    custom: { [key: string]: any; }
}

For a moment I was wondering if we can define the metadata as simple as

export interface NotebookCellMetadata {
    editable?: boolean;
    ...

    [key: string]: any;
}

which is convenient, but the catch is if we later on introduce new builtin metadata, we can't guarantee that it doesn't break existing notebooks. Considering that naming is hard, we are very likely to use names which are already used by Jupyter Notebook, the API ended up with the first proposal for now: every custom metadata should be set inside custom namespace.

@DonJayamanne
Copy link
Contributor

DonJayamanne commented May 13, 2020

Not really a fan of the following:

export interface NotebookCellMetadata {
    editable?: boolean;
    ...

    [key: string]: any;
}

Its not easy identify what's custom vs VS Code.
E.g. what if someone decides to add a custom metadata named editable and that means something completely different. I.e. we'd need to be sure that Jupyter Specification doesn't break VS Code because they decided to add some metadata.

Will pull master and try this out now.

@DonJayamanne
Copy link
Contributor

@rebornix
When copying a cell, will this metadata get copied across into the new cell?
If yes, then we have a problem, as we cannot use the metadata to uniquely identify a cell and map it to something in the extesion.

Feels like extension authors need a way to provide an identifier to a cell.

@dbaeumer
Copy link
Member

dbaeumer commented Jun 3, 2020

@jrieken any idea if this was tested or needs verification?

@jrieken
Copy link
Member

jrieken commented Jun 3, 2020

🤷 ping @rebornix

@rebornix rebornix added verification-needed Verification of issue is requested and removed verification-needed Verification of issue is requested labels Jun 3, 2020
@rebornix
Copy link
Member

rebornix commented Jun 3, 2020

We still have ongoing discussion about waht/how metadata are being copied to copy/paste. Reopen this and finalize in next iteration.

@rebornix rebornix reopened this Jun 3, 2020
@rebornix rebornix modified the milestones: May 2020, June 2020 Jun 3, 2020
@rebornix rebornix modified the milestones: June 2020, Backlog Jul 1, 2020
@jrieken jrieken added the api label Sep 15, 2020
@TylerLeonhardt
Copy link
Member Author

I think we can close this for now. The PowerShell extension already takes advantage of the custom metadata and it works nicely.

@github-actions github-actions bot locked and limited conversation to collaborators Feb 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api feature-request Request for new features or functionality notebook
Projects
None yet
Development

No branches or pull requests

5 participants