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

make KernelManager configurable for all who inherit JupyterConsoleApp #379

Merged
merged 2 commits into from
May 17, 2018

Conversation

mpacer
Copy link
Member

@mpacer mpacer commented May 15, 2018

No description provided.

@mpacer mpacer force-pushed the config_kernel_manager branch from d224555 to db8b3cf Compare May 15, 2018 20:53
@mpacer mpacer force-pushed the config_kernel_manager branch from db8b3cf to f1d8a95 Compare May 15, 2018 20:58
@mpacer
Copy link
Member Author

mpacer commented May 15, 2018

@rgbkrk @takluyver @Carreau @minrk Do any of you have any comments on this?

@rgbkrk
Copy link
Member

rgbkrk commented May 15, 2018

My only real question is if we have to bear in mind what the NotebookApp or others already use for this field. If it overlaps, does it maintain the same purpose? (I do realize the notebook app has the two related classes, one for the single base and one for the mapping kernel manager).

@mpacer
Copy link
Member Author

mpacer commented May 16, 2018

The NotebookApp doesn't use this field — it is using NotebookApp.kernel_manager_class which does not inherit from JupyterConsole and so would have no way to access JupyterConsole.kernel_manager_class.

As far as I can tell there are two main uses of the JupyterConsole application:

  • juptyer_console (which does not overwrite this attribute and so will inherit it appropriately)
  • qtconsole (which overwrites this attribute to be QtKernelManager thus will not be able to inherit this value)

Are there other known uses of the JupyterConsole application?

@Carreau Carreau added this to the 6.0 milestone May 17, 2018
@Carreau Carreau merged commit 2fae25b into jupyter:master May 17, 2018
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.

3 participants