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

Validation should not mutate arguments #235

Open
Carreau opened this issue Nov 16, 2021 · 1 comment
Open

Validation should not mutate arguments #235

Carreau opened this issue Nov 16, 2021 · 1 comment

Comments

@Carreau
Copy link
Member

Carreau commented Nov 16, 2021

Currently if a code cell has a missing id, it will silently add one to it. This is problematic for a few reasons.

  1. it mutates arguments.
  2. it is counter intuitive WRT the name of the function and the docs that says it raises if invalid.

1 + 2 create signing issues, indeed validate is called when writing and most notebook manger do :

  • compute signature
  • save (indirectly validate).

They assume that what you save is identical to what you give to nbformat, but as validate mutates things it is untrue.

Now with validate mutating, what is saved has a different signature than what was computed just above, and worse, as it mutates you may be scratching your head as to why.

I thin that validate should raise unconditionally if the notebook is invalid or requires modifications, and that for convenience you may want to have a normalise() function/method that you can call to do any mutations if you wish to.

As raising is likely to break a bunch of stuff, we have to have a transition plan. I suggest to emit a warning.

Carreau added a commit to Carreau/nbformat that referenced this issue Nov 16, 2021
Carreau added a commit to Carreau/nbformat that referenced this issue Nov 16, 2021
Carreau added a commit to Carreau/nbformat that referenced this issue Nov 17, 2021
@marscher
Copy link

this has been fixed in 5.6.0, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants