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

upper bound for pyzmq/jupyter_client for notebook<7 #202

Merged
merged 3 commits into from
Jul 28, 2023
Merged

Conversation

mcg1969
Copy link
Contributor

@mcg1969 mcg1969 commented Jul 27, 2023

See this PR and linked issues:
jupyter/notebook#6749

@mcg1969
Copy link
Contributor Author

mcg1969 commented Jul 27, 2023

FYI, these pins will be built into notebook 6.5.5 already. But we need these for older versions, otherwise conda is likely to downgrade notebook to get newer versions of jupyter_client and pyzmq.

@mcg1969
Copy link
Contributor Author

mcg1969 commented Jul 27, 2023

Notebook 5 already has an upper bound for jupyter_client but not for pyzmq. Talking with @RRosio it is my view that applying the pymzq pin is still worthwhile in that case.

Copy link

@markan markan left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

Copy link

@Jrice1317 Jrice1317 left a comment

Choose a reason for hiding this comment

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

LGTM

if name == "notebook":
replace_dep(depends, "tornado >=4", "tornado >=4,<6")
if int(version.split('.', 1)[0]) < 7:
replace_dep(depends, "pyzmq >=17", "pyzmq >=17,<25")
replace_dep(depends, "jupyter_client >=5.3.4", "jupyter_client >=5.3.4,<8")
Copy link

Choose a reason for hiding this comment

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

It looks like notebook version 5.0.0-6.0.1 ether pins to jupyter_client>=5.2.0 or doesn't pin at all. Do those need the upper bound as well? Or is some other constraint protecting us here?

Copy link

Choose a reason for hiding this comment

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

In practice most of these are built for old versions of python, but notebook-5.7.9-py38_0 might still slip through.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add that

@mcg1969 mcg1969 merged commit ddd7c3c into master Jul 28, 2023
1 check passed
@lorepirri
Copy link

I've deleted the branch and change the settings of this repo to do it automatically after merge

@lorepirri lorepirri deleted the notebook6 branch July 28, 2023 12:14
replace_dep(depends, "pyzmq >=17", "pyzmq >=17,<25")
replace_dep(depends, "jupyter_client >=5.3.4", "jupyter_client >=5.3.4,<8")
replace_dep(depends, "jupyter_client >=5.2.0", "jupyter_client >=5.2.0,<8")
replace_dep(depends, "jupyter_client", "jupyter_client <8")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pins added in response to comment

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.

4 participants