Add name to the kernelOptions

[joergbrech]

Fixes #59.

See also executablebooks/thebe#349.

Follow-up question: Can we have some kind of doctests verifying that all code blocks in the documentation don't error?

Edit from @choldgraf : i think this also closes #43 and closes #53

Edit: Closes executablebooks/jupyter-book#1173

[welcome]

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

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

[choldgraf]

Good catch - do you think we can deprecate kernelName or should we keep it?

For tests, i agree, but nobody has had the bandwidth to write them. I guess you could do it with playwright or something like this.

[joergbrech]

Good catch - do you think we can deprecate kernelName or should we keep it?

I honestly don't know 🤷 I wasn't brave enough to deprecate it.

[choldgraf]

@stevejpurves would you be willing to provide guidance on whether we can remove/deprecate kernelName?

[stevejpurves]

I would keep it -- I believe that a kernel launch can fail if the (default) named kernel is not available on the server/service, and if for some reason a kernel named python3 is not there then that would block usage, even though the public binder may have that kernel available.

[choldgraf]

@stevejpurves so name and kernelName serve two different purposes then? Could you link to a doc (or provide briefly here) so that we can cross-link in the sphinx-thebe docs to help others understand when/how to use?

[choldgraf]

ah looking at the comment in the linked issue, it seems that name is the new correct option after thebe 0.6, and kernelName is an old / deprecated option:

• New JupyterLab API for spawning kernels has a different config option name for which kernel to spawn thebe#349

Does that sound correct to you @stevejpurves ?

[joergbrech]

I am not entirely sure, but I believe this could fix executablebooks/jupyter-book#1173 also (with a sphinx-thebe release and a bump in the jb dependencies).

[choldgraf]

Yep i think you are right. Let's cross link that issue in the PR as well so that it closes it. We can make a release quickly!

[stevejpurves]

@choldgraf I can't actually find any reference to the two existing side by side outside of thebe.
AskernelName has no effect in the current thebe release, and as thebe is moving to not using the Juptyerlab sessions at all - it looks like deprecating kernelName would be ok here after all, and as a breaking change make it clearer for sphinx-thebe users to switch to /usename.

In the meantime, thebe 0.8.x needs an update to provide the backwards compatbility

[choldgraf]

Ah ok, thanks for this clarification - I'd then suggest:

• make kernelName rename itself to name if given, and raise a warning.
• Add a comment to that line that the 0.8 release of thebe will change its API. @stevejpurves it would be really helpful if you took a look at this extension just to recommend if there are any other things that will be deprecated)
• open an issue to track the future change

[joergbrech]

make kernelName rename itself to name if given, and raise a warning.

@choldgraf I am afraid I don't understand enough about the interplay of thebe/sphinx/jupyter etc. What do you mean by this? Doesn't sphinx-thebe just parse thebe-kernel from the Markdown metadata and copy it to the thebe html config? Where would kernelName be given, so that it can be renamed?

[choldgraf]

@joergbrech ah I'm being silly - kernelName is chosen by sphinx-thebe, not the user. I think we can just deprecate the use of kernelName entirely by deleting that line. I added comments to make that clearer.

I think we should still open an issue about the soon-to-be-deprecated-again API. @stevejpurves would you be willing to explain in a new issue what we'll need to change?

[stevejpurves]

@choldgraf yes I will - how about you go ahead and release this change first, then I can sit down and test the jb|sphinx-thebe|thebe together and make sure I have a clear picture.

[joergbrech]

@choldgraf just a friendly ping. Are we ready to merge?

[choldgraf]

Looks good, thanks!

[welcome]

Congrats on your first merged pull request in this project! 🎉


Thank you for contributing, we are very proud of you! ❤️

[choldgraf]

Yep - thanks for the ping. If any of you are interested in having merge rights on this repository, I am happy to provide them. I clearly do not have enough time to maintain this in addition to all the other stuff I have to do, and I don't want to block progress on myself!

[joergbrech]

Yep - thanks for the ping. If any of you are interested in having merge rights on this repository, I am happy to provide them. I clearly do not have enough time to maintain this in addition to all the other stuff I have to do, and I don't want to block progress on myself!

Thanks, but as this is just a small side project, I don't see myself contributing often. Also I know too little about the whole jupyter-sphinx-thebe ecosystem to be of much value. Once this is in jupyter book I will be happy for a while 🙂