Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork3.8k
Add option to hide native kernel sources#18155
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
Thanks for making a pull request to jupyterlab! |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Darshan Poudel <pranishpoudel10@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Not sure if storing all these kernel sources like this is ideal.
What do you think,@krassowski@martinRenou ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Agreed. I suggested in#18077 (comment) to make a JEP for it.
A hardcoded list is the only thing we can do for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
How was it generated? I guess a python script? Could JupyterLab run this script at build time or startup time to populate this json file from the actual kernel version available since we have a dependency on ipykernel anyways? Or if not, why is it a bad idea?
martinRenouDec 12, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Arjun generated this from the value ofsys.modules right after booting the kernel in both ipykernel and xeus-python, and merged the results.
I'm not sure I have an opinion about generating that list at build time using ipykernel, on one side it would make xeus-python behave a bit differently as we would see the xeus-python Python sources listed, but that's probably not too bad.
Also to note that it's unlikely that this lists get outdated on a fast occurrence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Makes sense, fine to keep as-is then, if this would take more time then I think the time is better spent prepping the JEP.
| .compositeasboolean; | ||
| if(hide){ | ||
| service.model.kernelSources.hiddenSources=filtersData; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Maybe this is implementation details, sorry if it is the case 😄, but I think that there is no need for settings the list of hidden sources from here.
The list of "native kernel sources" is (and will be) always static: currently it comes from a JSON file, and (maybe after a JEP), it will come from the debugger service. In any case, it should not change after initialisation.
When it will come from the debugger service, this statement will be removed, because the list will not be fetched from here, as far as I understand.
IMO, we should only set a flag here (e.g.hideNativeKernelSource), and set the list of native kernel source in the constructor of the model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for the comprehensive review@brichet, I'll update this today
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think this was not updated yet, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sorry for the delay I pushed a refactor in8bda2a2
Uh oh!
There was an error while loading.Please reload this page.
References
xref:#18077 (comment) 䚝 (comment)
This PR introduces a new boolean setting,
hideFilteredKernelSources, that controls whether filtered kernel sources are hidden automatically in the Kernel Sources panel.Code changes
User-facing changes
Screen.Recording.2025-11-27.at.15.13.43.mp4
Backwards-incompatible changes