-
Notifications
You must be signed in to change notification settings - Fork 425
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
papermill --prepare-only fails if kernel is missing #262
Comments
Currently, when there is no translator for the notebook's kernel, the installed kernel's kernelspec is used to determine the notebook language (and find an appropriate translator). Perhaps we could use the language specified in the notebook's metadata instead (right next to the kernel name), so that it can be parameterized despite the missing kernel. There could be an issue if a user has multiple kernels with the same name but different languages on different systems, but that sounds a bit far-fetched... There's also the case where the --kernel option is used. In that situation I think it'd be best not to rely on anything in the notebook metadata, as it has been overridden. I would continue looking for an installed kernel in that case. |
I think making the logic try language, then kernel name, then lookup kernel for language match would be ideal here. That way a user could register a new translator to the specific kernel/language definition in their ipynb if they really needed to execute without the kernel installed, but still respect custom kernels that define their language. |
So, right now the order of precedence is:
See translators.py: def kernel_translator(self, kernel_name):
kernelspec = get_kernel_spec(kernel_name)
if kernel_name in self._translators:
return self._translators[kernel_name]
elif kernelspec.language in self._translators:
return self._translators[kernelspec.language]
raise PapermillException(
"No parameter translator functions specified for kernel '{}' or language '{}'".format(
kernel_name, kernelspec.language
)
) Are you saying that you would change this to:
I assumed that kernel name came before language because translators for kernels are, in a way, more specific than translators for a language, no? |
Perhaps: If --kernel is specified:
Else:
|
Actually I think you're right about matching 1) kernel name then 2) language. Would swapping # 2 and # 3 from the else case make more sense? So we check for static values before requiring a kernelspec to be loaded? If --kernel is specified:
Else:
|
So I've given it a bit more thought and I'm starting to think this is more complicated than it needs to be, or should be. As far as I know, in normal use, the appropriate translator will be the same whether you look at the ipynb or the kernelspec. How about just using something like this:
I have trouble coming up with compelling user stories where it would make a difference to base the translator choice on the --kernel option or on the kernelspec. There would need to be a scenario where:
Basing the translator choice exclusively on the ipynb means that it will yield the same result on different machines. Also, I'm not sure we should change the scope of the --kernel option from "Name of kernel to run" to "Name of kernel to run OR to consider when preparing the notebook" |
I think the comment on "this is more complicated than it needs to be" is right. If a user really wants special translators they can deregister/register their own in place of the built-ins. Let's go with this latest pattern you outlined. |
PR merged, working on a final issue or two before cutting a release to add the changed behavior. |
When running with --prepare-only, I don't expect papermill to evaluate my notebook. However, if I attempt to process a notebook whose kernel is not installed in the current environment, I get a "No such kernel" error. Is this expected?
If I install the missing kernel, papermill completes as expected.
The text was updated successfully, but these errors were encountered: