-
Notifications
You must be signed in to change notification settings - Fork 294
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
Fix export to python script not appearing #5407
Conversation
kernelspec name includes python
if (kernelSpec?.name.includes(PYTHON_LANGUAGE)) { | ||
return true; | ||
} | ||
|
||
// Valid notebooks will have a language information in the metadata. |
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.
There's exceptions to this, sometime the kernelspec only has name
and displayname
@@ -109,6 +109,11 @@ export function isPythonNotebook(metadata?: nbformat.INotebookMetadata) { | |||
if (metadata?.language_info?.name && metadata.language_info.name !== PYTHON_LANGUAGE) { | |||
return false; | |||
} | |||
|
|||
if (kernelSpec?.name.includes(PYTHON_LANGUAGE)) { |
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.
This seems like it won't work consistently. What line here is causing the notebook to not be python otherwise?
@rchiodo its the next bit
Sometime kernelspecs don't have the |
Maybe there's more we can do then? I think the 'name' check for python will work in some cases, but may not. Could we not also check the 'executable' being run for the kernelspec? It should have 'python' in it somewhere. |
Codecov Report
@@ Coverage Diff @@
## main #5407 +/- ##
=====================================
- Coverage 73% 73% -1%
=====================================
Files 401 401
Lines 26580 26583 +3
Branches 3878 3879 +1
=====================================
- Hits 19510 19506 -4
- Misses 5454 5459 +5
- Partials 1616 1618 +2
|
|
||
if (notebook.metadata && isPythonNotebook(notebook.metadata)) { | ||
if (interpreter) { |
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.
What if we don't have the python extension installed?
I think this would be better off being an or with the previous condition.
* assume a notebook is a python notebook if the kernelspec name includes python * news * use intepreter to decide if its a python notebook * keep both conditions
Assume a notebook is a python notebook if the kernelspec name includes 'python'
For #5403
(and still works with #4965)
package-lock.json
has been regenerated by runningnpm install
(if dependencies have changed).