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

Drop ipykernel dependency #255

Merged

Conversation

SylvainCorlay
Copy link
Contributor

Closes #254.

@blink1073
Copy link
Collaborator

We should probably test against both xeus-python and ipykernel on CI.

@echarles
Copy link
Member

yep, I was thinking to unit tests that execute cells, so need kernel. +1 to test on both xeus-python and ipykernel on CI although it may sound weird to test something that we wouldn't ship.

@SylvainCorlay
Copy link
Contributor Author

We should probably test against both xeus-python and ipykernel on CI.

We could do that, although xeus-python is already continuously tested with jupyter_kernel_test.

@blink1073
Copy link
Collaborator

Okay, let's test against whichever one is currently shipped with JupyterLab then, and keep them in sync.

Copy link
Collaborator

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thanks!

@SylvainCorlay
Copy link
Contributor Author

Okay, let's test against whichever one is currently shipped with JupyterLab then, and keep them in sync.

We could work on testing a variety of language kernels at some point.

@blink1073
Copy link
Collaborator

Julia and R would be good for our namesake. 😀

Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

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

Good catch - thanks.

@kevin-bates
Copy link
Member

Hmm. It just dawned on me. How will the default kernel name be determined? It currently comes from MultiKernelManager.default_kernel_name and defaults to python3 when the trait isn't configured. jupyter_client does not depend on ipykernel either (only for testing), so we could theoretically encounter a condition where the default interaction with kernels now breaks.

@SylvainCorlay
Copy link
Contributor Author

How will the default kernel name be determined? It currently comes from MultiKernelManager.default_kernel_name and defaults to python3 when the trait isn't configured. jupyter_client does not depend on ipykernel either (only for testing), so we could theoretically encounter a condition where the default interaction with kernels now breaks.

I think that it is fine to default to python3. Erroring when attempting to start a notebook when no kernel is available is expected.

@kevin-bates
Copy link
Member

Understood. I'm just saying that previous to this change, there would not have been an error, so there's potential for users claiming a regression. Deployment manifests implicitly relying on that dependency (correctly or not) will need to be updated to include the installation of ipykernel now. That said, switching to jupyter_server is a good time to do this.

@SylvainCorlay
Copy link
Contributor Author

That said, switching to jupyter_server is a good time to do this.

Yes, that's why I did not open the PR to the notebook server.

We are also removing some of these arbitrary dependencies in ipywidgets 8.0...

@Zsailer Zsailer merged commit 19e887c into jupyter-server:master Aug 4, 2020
@blink1073 blink1073 added this to the 1.0 Release milestone Sep 17, 2020
Zsailer added a commit to Zsailer/jupyter_server that referenced this pull request Nov 18, 2022
hMED22 pushed a commit to hMED22/jupyter_server that referenced this pull request Jan 23, 2023
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.

Drop ipykernel dependency
5 participants