Fix part of the problem with notebook losing trust. #6240
Conversation
Carreau
force-pushed
the
trust-issues
branch
from
3a36d91
to
2b321ec
Compare
See jupyter/nbformat#236 and https://github.com/jupyter/nbformat/issue/235. This mostly affects frontends that do not always add ids to cells, like classic notebook, and that issue is not limited to cells ids, but also to empty cells, that may not have/need a trusted field. But that is another issue.
Carreau
force-pushed
the
trust-issues
branch
from
2b321ec
to
7fb50a8
Compare
There was a problem hiding this comment.
Thanks @Carreau, just had a couple of comments/suggestions.
| @@ -473,6 +496,8 @@ def save(self, model, path=''): | |||
| try: | |||
| if model['type'] == 'notebook': | |||
| nb = nbformat.from_dict(model['content']) | |||
|
|
|||
| nb = normalize(nb, nb.get("nbformat"), nb.get("nbformat_minor")) | |||
There was a problem hiding this comment.
Is there a use case where we would NOT want to pull the version info from the notebook? If not, I think it would be cleaner to access the version info within normalize().
| from copy import deepcopy | ||
|
|
||
| def normalize(nbdict, version, version_minor): | ||
| nbdict = deepcopy(nbdict) |
There was a problem hiding this comment.
Could we make the deepcopy conditional on version? Not sure how many pre-cell-id notebooks are out there but this would save some time/space for them.
| def normalize(nbdict, version, version_minor): | ||
| nbdict = deepcopy(nbdict) | ||
|
|
||
| if version >= 4 and version_minor >= 5: |
There was a problem hiding this comment.
This needs a minor adjustment so lower minor versions are included for future major versions.
| if version >= 4 and version_minor >= 5: | |
| if (version == 4 and version_minor >= 5) or version >= 5: |
|
Closing as this would be something to implement in Jupyter Server, if still applicable: https://github.com/jupyter-server/jupyter_server Also the Notebook 7 trust mechanism follows what JupyterLab and Jupyter Server implement. |
See jupyter/nbformat#236 and
https://github.com/jupyter/nbformat/issue/235.
This mostly affects frontends that do not always add ids to cells,
like classic notebook, and that issue is not limited to cells ids, but
also to empty cells, that may not have/need a trusted field.
But that is another issue.