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

Add autoStartDefault to getKernelPreference #12793

Closed
wants to merge 3 commits into from

Conversation

aditya211935
Copy link
Contributor

References

Code changes

getDefaultKernel function requires autoStartDefault argument to return defaultKernel. This in turn is used to start the notebook with default kernel if no kernel is specified.

However, autoStartDefault property is not defined anywhere execept in the original interface.

readonly autoStartDefault?: boolean;

I've added autoStartDefault property to NotebookWidgetFactory as true. This fixes the problem.

User-facing changes

Previously, on clicking New Notebook from file explorer's context menu, the notebook would be opened without any kernel.
With these changes, the notebook opens with the default kernel.

Screen.Recording.2022-07-12.at.12.59.09.AM.mov

Backwards-incompatible changes

@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@welcome
Copy link

welcome bot commented Jul 11, 2022

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@aditya211935
Copy link
Contributor Author

@afshin @fcollonval Any updates?

@krassowski
Copy link
Member

This would also alleviate a problem currently faced for users of krassowski/jupyterlab-voice-control#5 extension. I am tagging it as a enhancement and adding to 4.0 milestone (so we have more time in case if someone comes back with a strong case for the existing behaviour).

@aditya211935 thank you for your contribution. Next time to get the PRs faster through the review please first open an issue describing the problem that you are trying to fix, as recommended and explained in the contributing guideline.

@krassowski
Copy link
Member

I think that this PR is compatible with other requests to improve the kernel startup like in: #12019 in principle. Recently a new draft PR appeared which touches similar ideas in a different user experience path: #12858 by @a3626a.

@a3626a
Copy link
Contributor

a3626a commented Oct 28, 2022

@krassowski

I think that this PR is compatible with other requests to improve the kernel startup like in: #12019 in principle. Recently a new draft PR appeared which touches similar ideas in a different user experience path: #12858 by @a3626a.

I changed my draft to PR, now. :)

@krassowski
Copy link
Member

Would you mind updating with the latest master branch (rebase or merge) so we can make sure the tests are passing? At the time of last commit there was something failing with the UI tests (unrelated to this PR).

@krassowski
Copy link
Member

Thanks! It looks like the visual regression tests are failing and I believe that this is relevant to this change. This is because they await for a dialog window about kernel selection, which is being skipped with this PR.

@krassowski
Copy link
Member

I think that autoStartDefault should be exposed in notebook settings. For now we could default it to false (this way tests would not need updating) and if we have more interest, change the default to true in a future release.

@aditya211935
Copy link
Contributor Author

@krassowski Sure, will add autoStartDefault to notebook settings and default it to false.

@fcollonval fcollonval added the api-change A change that should be accompanied by a major version increase label Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change A change that should be accompanied by a major version increase enhancement pkg:docregistry pkg:notebook
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants