Make comm manager (mostly) independent of InteractiveShell

[takluyver]

This makes it possible to use comms from wrapper kernels, without instantiating the full IPython shell machinery.

The one remaining place where we need a reference to shell is to fire pre_execute and post_execute hooks (which are needed to get mpl figures right). This is a pure IPythonism, that it should be safe to ignore if shell is not set.

Ping @dsblank - you were asking about this in #6314.

[takluyver]

I'm not entirely happy about using this private attribute - I suspect there should be a public API on Kernel to get the parent header.

[takluyver]

Custom kernels would use this by instantiating CommManager(kernel=self), with no shell parameter.

[dsblank]

Great! Am now checking this out...

[takluyver]

Hmm, I've missed a bit in IPython.kernel.comm.comm.Comm.open() and .close() - the comm uses the shell to get a reference to the comm manager. I'll keep thinking about that.

[dsblank]

@takluyver I see... comm.open() and comm.close() both call get_ipython() to get access to the CommManager. Those get_ipython() calls should probably be calls to self._shell_default(), but there should really be a self._comm_manager() which we could overload, right?

[takluyver]

@dsblank I've just rewritten that to use self.kernel. When you instantiate CommManager, you will need to attach it to your Kernel subclass as kernel.comm_manager.

[takluyver]

This now makes an architectural change for IPython: it's the kernel that has the comm manager, whereas previously it was the shell.

[dsblank]

This looks cleaner in that there isn't the need to call out to
get_ipython(). I do see a few "get_ipython()s" sprinkled throughout the
codebase, but this looks like a good start to try working on widgets from
other Python-based languages. Thanks!

On Wed, Sep 24, 2014 at 8:50 PM, Thomas Kluyver notifications@github.com
wrote:

This now makes an architectural change for IPython: it's the kernel that
has the comm manager, whereas previously it was the shell.

—
Reply to this email directly or view it on GitHub
#6540 (comment).

[takluyver]

You're welcome :)

[minrk]

you still need to check if self.kernel is None

[takluyver]

This should never be reached if kernel is None.

[minrk]

It can be during traitlet initialization. Setting a traitlet triggers its _default method (I think in order to determine if _changed should fire). So CommManager(session=foo, kernel=k) can trigger this call with kernel=None, depending on dictionary ordering.

[takluyver]

Since kernel is a singleton, the default for .kernel should retrieve the instance anyway. Though if people instantiate it by calling the class, that doesn't register the singleton instance. I guess I'll put in the safeguard...

[takluyver]

I am happy with the current state of this if people want to review/merge it.

[Carreau]

Looks good. Merging soon is no objections.

[jdfreder]

Thanks @takluyver !