Hi :) Not necessarily a "bug", but more of a suggestion for the API - currently, nbformat.v4.upgrade outputs None if the notebook is already in v4, which might be a reasonable thing to do (to indicate that no change is necessary), but note that papermill does the following
# Upgrade the Notebook to the latest v4 before writing into it
nb = nbformat.v4.upgrade(nb)
without a check for None. This means if the notebook was already in v4 papermill breaks (which I discovered due to, well, papermill breaking!
So the solutions are either to modify papermill, or (and this is where I'd ask for your advice), maybe the upgrade function should just return nb? For what it's worth, this is the current code (nbformat/v4/convert.py, line 73)
elif from_version == 4:
if from_minor == nbformat_minor:
return
but when I delete the return line, Copilot prefers to fill in
elif from_version == 4:
if from_minor == nbformat_minor:
return nb
which might be a somewhat objective way to show that this is the more intuitive API?
Hi :) Not necessarily a "bug", but more of a suggestion for the API - currently,
nbformat.v4.upgradeoutputs None if the notebook is already in v4, which might be a reasonable thing to do (to indicate that no change is necessary), but note thatpapermilldoes the followingwithout a check for None. This means if the notebook was already in v4 papermill breaks (which I discovered due to, well, papermill breaking!
So the solutions are either to modify papermill, or (and this is where I'd ask for your advice), maybe the upgrade function should just return nb? For what it's worth, this is the current code (
nbformat/v4/convert.py, line 73)but when I delete the return line, Copilot prefers to fill in
which might be a somewhat objective way to show that this is the more intuitive API?