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

pre-commit hook : nbformat is inconsistent locally and on CI #715

Closed
jturner65 opened this issue Jan 14, 2021 · 7 comments
Closed

pre-commit hook : nbformat is inconsistent locally and on CI #715

jturner65 opened this issue Jan 14, 2021 · 7 comments
Milestone

Comments

@jturner65
Copy link

Hello
I am a developer on Habitat-Sim. I have just started having this problem (yesterday everything was working properly). When I run pre-commit locally on our codebase via :

pre-commit run jupytext-sync --all-files

all colabs in the project pass, but when I run it on CI, it seems to be attempting to upgrade the nbformat minor version from 4 to 5, and inserting synthesied v4.5-compatible ID values in each cell.

Here's a link to the CI with the error.

I am using jupytext 1.6.0 and on my local python is 3.8.5.

@mwouts
Copy link
Owner

mwouts commented Jan 14, 2021

Hi @jturner65 , well I was glad to learn about the new id field of cells in nbformat 4.5... I've just read https://jupyter.org/enhancement-proposals/62-cell-id/cell-id.html (but maybe that is not the most up-to-date documentation)

I agree with you, using jupytext should not cause a change in the notebook format. I'll see how to add a test for that.

Is it an option for you to use jupytext>=1.7.1? In that version we fixed the issue #671 (missing attachments in markdown cells), and that might have improved the stability of the notebook format version in round trips.

Also cc @Skylion007 as he knows the Habitat project better than me.

@mwouts mwouts added this to the 1.9.2 milestone Jan 14, 2021
@mwouts
Copy link
Owner

mwouts commented Jan 14, 2021

Oh I do have the same issue on my recent PR, cf. #709 for instance

@jturner65
Copy link
Author

Do you think this might be something with python 3.8.x?

@mwouts
Copy link
Owner

mwouts commented Jan 14, 2021

The Jupytext plan that fails uses nbformat==5.1.0, and I can reproduce the errors locally once I do

pip install 'nbformat==5.1.0'

This is the version that introduces the nbformat version 4.5, cf. https://nbformat.readthedocs.io/en/latest/changelog.html

So probably the quickest fix here will be to require nbformat<=5.0.8 on the CI.

@Skylion007
Copy link
Contributor

This is going to cause issues with #698

@mwouts
Copy link
Owner

mwouts commented Jan 14, 2021

This is going to cause issues with #698

Yes and no... sure any PR with no upper limit on nbformat will fail, but the quick fix d09af23 will be easy to merge

@jturner65
Copy link
Author

Since the suggestion offered seems to have addressed the issue, I'll close this. Thanks a lot folks! 👍

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

3 participants