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

Remove custom entry from Jupyter Notebook & Cell Metadata #205637

Closed
5 tasks done
DonJayamanne opened this issue Feb 20, 2024 · 5 comments
Closed
5 tasks done

Remove custom entry from Jupyter Notebook & Cell Metadata #205637

DonJayamanne opened this issue Feb 20, 2024 · 5 comments
Assignees
Labels
debt Code quality issues notebook notebook-ipynb on-testplan papercut 🩸 A particularly annoying issue impacting someone on the team
Milestone

Comments

@DonJayamanne
Copy link
Contributor

DonJayamanne commented Feb 20, 2024

This is an unnecessary layer
Everywhere in code we need to look at custom, when there's no such thing in Jupyter (specification) metdata.
This is a carry over from the old VS Code notebook Notebook format.
All metadata had to be stored under custom and this was never migrated when custom in core was removed.

This shows up in SCM view as well
I.e. when using the diff view, the custom node is displayed, and this is wrong, as there's no such thing in the spec.

Removing this will:

  • Improve SCM
  • Make it easier for extensions to access the metadata
  • Ensure the API lines up with the Jupyter specification

Tasks

  1. debt
    DonJayamanne
  2. insiders-released notebook notebook-diff
    DonJayamanne
  3. debt on-testplan
    DonJayamanne
  4. area-notebook
    DonJayamanne
  5. bug on-testplan
    DonJayamanne
@DonJayamanne DonJayamanne added the debt Code quality issues label Feb 20, 2024
@DonJayamanne DonJayamanne self-assigned this Feb 20, 2024
@DonJayamanne DonJayamanne added notebook papercut 🩸 A particularly annoying issue impacting someone on the team labels Feb 20, 2024
@DonJayamanne
Copy link
Contributor Author

@rebornix /cc
We will need to coordinate this with partner teams that read/write ipynb notebooks

@DonJayamanne DonJayamanne changed the title Remote custom entry from Jupyter Notebook Cell Metadata Remove custom entry from Jupyter Notebook Cell Metadata Feb 20, 2024
@rebornix rebornix added this to the On Deck milestone Feb 22, 2024
@DonJayamanne
Copy link
Contributor Author

@davidanthoff
From what I can tell, the only place where the Notebook or Cell metadata is accessed, is in the notebookFeature.ts file.
My plan is to submit a PR that would make a change to support both the old custom and the new format.
This way, it would be backwards compatible if and when we move away from the custom property.

Should be easy enough to create a single function that returns the metadata and use that everywhere.

How does that sound?

@DonJayamanne
Copy link
Contributor Author

@colombod

Looking at the code in Polyglot Notebooks (Interactive)
I can see it is used in a large number of places notebookControllers.ts, metadataUtilities.ts, notebookSerializers.ts, etc.
I think the best and safest way to make such a change in your repo would be to have an if condition that conditionally uses custom based on the version of VS Code being used.
From what I can tell, tehre are a number of files that would be effected, hence the change would not be as simple/straightforward, happy to discuss offline.

@davidanthoff
Copy link
Contributor

@DonJayamanne that definitely works!

@DonJayamanne
Copy link
Contributor Author

Closing as done.

@microsoft microsoft locked and limited conversation to collaborators Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues notebook notebook-ipynb on-testplan papercut 🩸 A particularly annoying issue impacting someone on the team
Projects
None yet
Development

No branches or pull requests

4 participants