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 pin on nbconvert to upgrade from 6.5.3 to 7.16.4 #117

Merged
merged 4 commits into from
Jun 25, 2024
Merged

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Jun 15, 2024

Bumps nbconvert from 6.5.3 to 7.16.4.

Fixes #116.

@weiji14 weiji14 self-assigned this Jun 15, 2024
Copy link

Binder 👈 Test this PR on Binder

@weiji14
Copy link
Member Author

weiji14 commented Jun 15, 2024

/condalock

conda-linux-64.lock Show resolved Hide resolved
environment.yml Outdated
@@ -45,7 +45,7 @@ dependencies:
# - jupyterlab-plotly~=5.18.0 # Outdated accoring to jupyterlab
- jupyterlab_pygments~=0.3.0 # To bring extension uptodate
- jupytext~=1.16.1
- nbconvert==6.5.3
- nbconvert~=7.16
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've set the pin to ~=7.16 instead of ~7.16.4, so this should allow any versions of nbconvert >=7.16.0, <8.0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new constraint solves my specific problem. You may want to consider whether a constraint on nbconvert is just going to cause future trouble. The package is required by jupyterlab via jupyter-server, so why not just let the dependency resolver choose a version (by removing nbconvert as an explicit entry in environment.yml)? Either way, thanks for the fix!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, jupyter_server on conda-forge depends on nbconvert-core, not nbconvert (which consists of nbconver-core + nbconvert_pandoc).

I do see that voila (added last month in #114) depends on nbconvert >=6.4.5,<8, so we could definitely remove nbconvert and let the version be resolved by whatever voila needs. The original pin on nbconvert==6.5.3 was added in the first version of CryoCloud at 2554461/#2 back in Nov 2022, and it doesn't seem like there was a strong reason to pin to any specific version, it was probably just the latest version at that time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I've removed the explicit pin on nbconvert at commit 78fcba9. Just need someone to approve and I can merge this in.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had no idea conda-forge would change the dependencies from what the package developers specify. Risky, but in this case looks safe, I guess, if confusing. All sounds good to me.

Copy link
Member Author

@weiji14 weiji14 Jun 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the naming is a little confusing, but see conda-forge/nbconvert-feedstock#72 for context. If I'm not mistaken, nbconvert-core on conda-forge is equivalent to nbconvert on PyPI without optional dependencies (see the recipe/meta.yaml file). I think they split out nbconvert-pandoc because pandoc is GPL licensed, so packaging both nbconvert (BSD-licensed) and pandoc might have GPL licensing implications.

@weiji14
Copy link
Member Author

weiji14 commented Jun 17, 2024

/condalock

@weiji14 weiji14 marked this pull request as ready for review June 17, 2024 23:38
@weiji14 weiji14 requested review from betolink and tsnow03 June 17, 2024 23:40
@weiji14 weiji14 changed the title Bump nbconvert from 6.5.3 to 7.16.4 Remove pin on nbconvert to upgrade from 6.5.3 to 7.16.4 Jun 18, 2024
@tsnow03 tsnow03 merged commit 72600dd into main Jun 25, 2024
1 check passed
@tsnow03 tsnow03 deleted the nbconvert7 branch June 25, 2024 03:53
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

Successfully merging this pull request may close these issues.

pinned version of nbconvert has problems with HTML blocks in markdown
4 participants