-
Notifications
You must be signed in to change notification settings - Fork 146
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
Jupyter kernel on vscode gives error about variables declared on above cells #932
Comments
@nayeemrmn could you take a look what's going on here? |
This will happen in the deno language server isn't being used - try adding It looks like you can also put your notebooks in a specific directory and add that directory to |
But I would say it should be safe for the extension to always use the deno language server for a notebook that has a deno kernel selected. |
I agree. Specifically because I don't want deno to be enabled on the global settings and I also don't want to save the notebook file sometimes. |
I'm also seeing this issue and have |
i'm wondering how this could be solved implementation-wise. my naive first impression was:
|
I forgot to give an update here, this is blocked by gluon-lang/lsp-types#268 which implements this API in the protocol https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#notebookDocument_synchronization. This provides the minimum of getting the accurate order of the cells etc. After that it's still going to be difficult and we'd likely have to do what @scarf005 proposed above. On the surface this isn't a problem with VSCode's built-in node TS server because it has notebook cells in script mode, where all vars are global. We might alternatively end up doing something similar. |
Would this PR help somehow? denoland/deno#21518 |
Just as a reminder, the code does work as expected, this issue is just about the lsp giving wrong diagnostics, but if you run the cell it will work. |
Unfortunately, even if it does execute, it's frustrating to lose all type information and be downgraded to |
Anyone have a work-around for this? As it stands, it's nearly unusable. :( |
This would be great to have. I've read the comments:
So wondering can import_maps be used to propagate type information across the cells? So far i'm leveraging deps.ts and it's not ideal as it still complains about unused values etc, but at least i'm getting the intelliSense.
|
@nayeemrmn gluon PR has been merged can this be resumed |
on top of that it'd be nice to have chrome-devtools-like object inspector (that expands as you click on the items) in notebook cell output |
You can already use rich display for that if you want https://deno.land/x/[email protected] and build custom formatters. |
The irony is that because the So my temporary workaround is...
{
"deno.enable":true,
"deno.disablePaths": [
"./notebooks"
]
}
|
@ple1n Can you try with "deno.path":"/home/phy/.cargo/bin/deno" ? Thanks |
why does deno exit. it crashed? and what is that object serialize it and print it ig. |
@ple1n Save this as connection_file Then launch Is deno exiting after that? |
|
I killed that process. It seems to be some left-over process that occupied the ports. And it worked |
I'm glad to hear that! @ple1n Thanks for your interest |
I mean how should the auto completion for that object be done. seems like a non trivial issue. infer the typing based on the object and send it back to lsp or such? and is that within the scope of your fork/pr / requiring deno-team intervention |
I have no idea how something like that could be done. It certainly wasn't on my to-do list for this project, but I'm open to helping in any way I can. |
It conflicts with other instances of the extension, ie I cant run multiple notebooks @redking00 |
@ple1n There is no problem running multiple notebooks on the same VSCode instance so I assume you mean multiple VSCode instances. It's probably the same problem with the ports. I have to add a random initial offset. |
@ple1n Just updated to v0.0.5. Can you check it?. I tested two VSCode instances with no problem. |
Yes exactly it pops up the error every second I write the sql statement (obviosuly it was kinda sending the sql to LSP for some reason). I cant suppress the notification it gets annoying |
no. it errors elsewhere too, pops up every key-hit |
|
and as predicted, by intuition. it disappears after reloading window |
For those interested in a quick test on the lsp thing... a codespaces template. @nayeemrmn , @bartlomieju, Any interest in this or do I keep the fork? Maybe it can be included as an opt-in setting for notebooks users until the real lsp is ready. |
check my project out https://github.com/planetoryd/drugbankdb I've been using this notebook feature with SurrealDB for data exploration. Its publicly available drugbank data you just need an account to download. I am not associated with anyone else its just for personal needs. The alternative is to use ELK stack, which does not work with graph data yet. Or use MongoDB stack, which did not work out for me, or has good features paywalled. And I am NOT gonna use python+pylance for surrealdb. It will suck. |
Thanks @redking00, @nayeemrmn is probably 50% done with the LSP integration at this point, but we have a few more pressing issues to fix before then. I'm not super keen on bringing a lot of new functionality into the extension as we try to keep it slim and offload most of the work to the LSP so it's easily testable. So I'd say it might be better to keep the fork going for now. |
@bartlomieju, @nayeemrmn, |
@redking00 I might be missing some crucial detail here - maybe you could open issues in the Deno repo with suggestions for the kernel and the serializer? |
@bartlomieju Both are vscode stuff. |
To Reproduce
Expected behavior
None errors
Screenshots
Versions
vscode: 1.82.2 deno: 1.37 extension: v3.23.1
@bartlomieju
The text was updated successfully, but these errors were encountered: