Add language support for notebook cells #236
Comments
|
@tgodzik This is something that I'd love to have in metals! Depending on how hard it is, I would consider a tilt at it :-)... I've done some experiments in the past, which prove that one can indeed run scala in a VSCode notebook, simply by wheeling in the Almond kernel. However, it's a "raw" experience right now. I hope, that could be improved in a couple of ways, which I believe to be 'independent'.
Is this something you'd devoted any thought to? If so, is the above rational? On a scale of 1-10, how hard would you perceive these problems to be? Something much above 5, and it's likely I'm out of my skill zone, sadly. If they aren't too bad, would you be willing to guide an effort along these lines, and / or set out a solution sketch? I'm not familiar with the project, so if you had a hint on where to look, it would be great, or any other thoughts? |
|
@Quafadas thanks for showing the interest! This is actually part of GSoC projects this year, but there it's possible it will not be picked. I will let you know once we have the results.
It already recognises the notebook cells written in Scala from what I've seen, but there is no support for the particular URI, which we would need to work on (we get the exceptions as shown in the description)
I might be possible to execute almond with the proper classpath and settings to make it work, but I am not familiar with the almond API right now.
I think it's around 5, but mostly it would be getting all the existing tools to work together, so a bit of things to figure out and change, but nothing that would require large scale rewrites.
I will let you know if this project gets to be a part of gsoc, if not we can try and figure it out together 😄 |
Ohhhh... excellent. Thanks for taking the time to write back. I'll keep my fingers crossed that it gets picked up by that project! |
This project hasn't been picked for GSoC, so if you are interested in working on it that would be awesome! I can try and provide some pointers, but generally I think the work you already did could be automated almond setup plus making sure that the notebook file provider works (there might be already an implementation for Java, but not sure.) |
|
*This project hasn't been picked for GSoC *
Oh no!
I’m away right now, so will write back in a week or so. I’d like to have
a think before committing to spending a lot of time, and probably do need
to figure out just how long it might take. I do think this would be a very
cool thing to have though!
…On Mon, 23 May 2022 at 10:06, Tomasz Godzik ***@***.***> wrote:
This is actually part of GSoC projects this year
Ohhhh... excellent.
Thanks for taking the time to write back. I'll keep my fingers crossed
that it gets picked up by that project!
This project hasn't been picked for GSoC, so if you are interested in
working on it that would be awesome! I can try and provide some pointers,
but generally I think the work you already did could be automated almond
setup plus making sure that the notebook file provider works (there might
be already an implementation for Java, but not sure.)
—
Reply to this email directly, view it on GitHub
<#236 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AF57BUDQS3VTDED6VPKREP3VLM36XANCNFSM5GLK7ARQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I think this one can be done iteratively, and I can fill in any missing pieces if it turns out you don't have time. Also, it's totally fine if you at some point decide that you can't work on it further. |
|
@tgodzik So I'm definitely up for this, although I have no experience with this sort of tooling. Should be fun. Sadly however for "reasons", I need to ban myself before mid-July - I have something else to focus on that I can't drop. I will spend some time now, trying to get started with changing metals and getting over the "contributor getting started" hump. Hopefully, I'll be posting up here around mid-July / early August, when I think I can allocate some time to this (if it's still open - which weirdly, I hope it is!). |
|
@tgodzik I hope to have some time over the coming weeks to look at this... my plan over the coming days is to look at getting started with changing metals and then go from there... |
|
Sure, I also mentioned it as a possible student project at EPFL, but that would be in septemeber and we can remove it from the list if you manage to take a look at it. Similar to GSoC this might also not be picked up at all. Some pointers:
|
|
@tgodzik So, I have invested a little time and sat down with this, and have (already!) reached the stage at which I'm not sure how to proceed. This is complex for me! For the sake of transparency, here is my branch. Changes thus far are basically trivial - add an almond version, and follow the pattern set out in the download methods. To be clear, it doesn't work! Because that artefact, doesn't exist. Maven Given that maintenance of the almond project itself is somewhat beyond the scope of this issue... that has some implications. Here are the things I'm now unsure about.
What (I think) about the above... I hope I'm not heading in the wrong direction
If the idea was instead to use metals to construct an almond launcher, which would then be launched as an independent service, then I guess this download step would be done rather differently by using coursier to bootstrap a launcher? I am assuming, that an "independent launcher" strategy, means that it would ultimately be very difficult to get the class paths of our build into the notebook...and hence that it is not the right direction of travel? I'm sorry this post is so long... |
That's a separate issue altogether, I can try ask about it. We could just have the latest supported version specified similar to how we deal currently with Ammonite. We can show a warning if a version is not yet supported
We don't want to always download all possible dependecies especially if we would need to download it for each Scala version, which would mean downloading every Scala version for the user even if they don't need it. The download step is used to avoid that.
We could use ShellRunner for running the specific kernel. When is the kernel actually needed? Only when running?
We can put that information in the build.sbt and show the users proper warnings. |
|
This is potentially a very useful note microsoft/vscode-jupyter#10813 https://jupyter-client.readthedocs.io/en/latest/kernels.html#kernel-specs microsoft/vscode-jupyter#10151 In summary; this now all looks tractable, if I can figure out how to bootstrap almond out of the coursier API. The first link says VSCode respects the kernel specs. The second link sets out the kernel specs, and suggests that a JSON file which allows one to, for example set environment variables for the notebook process. The third link sets out how one may influence the class path through setting environment variables. |
|
I made a slight change (pushed to branch above) to the shell runner to allow it to accept extra repositories when downloading dependancies, because almond has a dependancy on JVM repr which appears only to be available on jitpack. Easily solved. In the metals test project, if I fire up a console and run this script, it downloads the key almond kernel dependancy. The last line even installs the kernel in (apparently)... the right place. This gets tantalisingly close. It places the kernel in the right place, and it's even found by VSCode! There are two problems;
So what I think is happening, is that normally, coursier is bundling together all he stuff the JAR needs to launch vanilla almond. That's not gonna work for us. Based on my google-fu, I guess that I now need to tamper with class paths... I do not know much about class paths. I don't even know if that's embarrassing or not :-(. Can I get the class path of the project metals is working on before using the shell runner? Any ideas? |
Actually,
Och, that is interesting, so |
Ooooo, okay, let me try to figure that out.
Yes - I believe because it's ultimately VSCode which is going to launch the kernel, not us! I'm thinking that we should use metals to "install" the right version of the kernel. The massive + point about "installing", is that it puts it "in the right place", and VSCode finds it and we're inside that infrastructure for free... basically we take ruthless advantage of all the work already done in Almond on that front :-D! The rather negative point, is exactly that almond requires you to give it the command to start the kernel, when you install. I can imagine two solutions. Solution 1 Figure out the minimal set of classes to launch the kernel per scala version, and feed it those. Solution 2 We're in metals, right? So if can we get the class path of the project that the user is working on (during import build step perhaps?), and install the kernel at that moment? If so then I think it's a slam dunk. We
But at that point, the kernel has the project context - so it ought to able to use, whatever is on that class path? Which is kind of the big goal! Here's an example of the important kernel.json file. If no other classes are bundled, the launcher itself is like 200Kb, so I'm relaxed about having a few of them! I believe the Jupiter spec will replace the "connection file" variable with whatever VSCode sends it. |
|
Rather a long message with some big leaps. Did it make sense? Still thinking. |
That makes sense!
Actually, maybe what the launcher needs is the classpath of the project? Or classpath + kernel jar? But anyway we should be ok installing one kernel per project (lazily if possible) and using the project name (or target ID), which shouldn't be a big issue. |
I think it's this... or at least that'll be my next attempt! I'll aim in this direction. I'm probably not going to look at this again for a few days, but would you be able to point me at
|
You can find an example of how we do it in https://github.com/scalameta/metals/blob/main/metals/src/main/scala/scala/meta/internal/metals/JavaInteractiveSemanticdb.scala#L55 |
|
@tgodzik Hi ... thankyou again for taking the time to write back and help... that looks perfect. still a bit delayed... and trying to find time a block of time to look at this again because I'm pretty excited about it... if I get an actual proof of concept working, I guess that I will struggle to get it to the stage where it works properly inside metals... but first things first... baby steps! |
|
If you have anything ready, you can send me a diff and I can offer some suggestions! |
|
@tgodzik I haven't managed to spend enough time with this, but here is an initial (horrible) take on this. Which currently, doesn't work. scalameta/metals@main...Quafadas:metals:jupyter_notebooks scalameta/metals@main...Quafadas:metals:jupyter_notebooks The idea is to add a command to metals "setup notebook kernel for this project". Which I hope would do exactly what is says on the tin. I think that this is along the right lines, but has the following problems, in order of "I don't know how to solve this".
Currently, it's all run synchronously. I believe that should be tidied up and put in a future context, but you can see, that my progress is not rapid. Unfortunately. |
No worries, it's great to hear that you managed to work on it! You could create a PR inside your repository and maybe this way we could comment and discuss specifics of your changes? |
|
I've created a PR... I hope to the right place... both marked WIP |
|
I wrote some words about how can we add language support for notebooks TODOs
Enables metals-vscode to fire
|
|
@tanishiking I'm not familiar with how notebook cells work in LSP. Will all LSP messages need to have their incoming URIs translated into a local filesystem version (or some more extensive mapping info)? And then will these local filesystem URIs have to be translated back into a notebook cell URI when the replies are sent? |
Yes, I think all the LSP requests that requires semantic information from compiler / SemanticDB requires translation from incoming URI -> local filesystem version. For example, consider we have the following two cells in a notebook. // cell1
def add(a: Int, b: Int): Int = a + b
// cell2
add(1, 2)cell2 itself isn't valid (both syntactically and semantically invalid). So, Almond / Ammonite translate the whole notebook into a combined compilable Scala source file (that is local filesystem version) (I guess). I think all the compiler related analysis should be done on that combined compilable Scala source file. However, I don't think we need to support all the LSP requests / notifications for notebooks (for example,
Yes, because LSP client doesn't recognize the "local filesystem URI". LSP server need to translate back to the notebook cell URI and position. |
|
@tanishiking I've had to do something similar here to allow for a simple jar filesystem and translate something like See textDocument/implementation as an example All messages get their URIs translated in and out but it's only file-to-file rather than file-to-multiple-cells. If notebook cells need URIs translated on (nearly) every message then maybe we should consider the needs of this feature request and scalameta/metals#4114 together. The other option for the jar file system is this PR which creates a java filesystem for the I thought this option was a lot cleaner as it didn't interfere with the main codebase as much but my implementation was slow and I got bogged down in |
|
@tanishiking I'm not sure how relevant this is to the discussion, but I had a look at almond 0.13.1 (which is awesome) and following the instruction here https://jupyterlab-lsp.readthedocs.io/en/latest/Configuring.html with metals 0.11.8, produced this screenshot;
Does anyone know - is that screenshot proof that juypter is is talking to metals LSP? Or how I might be able to tell? |
|
Och, that's interesting. I wonder if they translate all the cells to a single document automatically 🤔 Maybe we could copy over their solution? @alexarchambault did you maybe work on that at any point? |
|
It looks like standard completion (the Ammonite one). LSP isn't involved. |
|
Interesting! but
|
Added a design document for scalameta/metals-feature-requests#236 This document is for - design review the architecture before going further - make sure we are on the same page about the goal, design, and requirements for this feature
|
added design doc for this feature scalameta/metals#4434 |
Is your feature request related to a problem? Please describe.
Currently, when using notebook cells in VS Code, Metals is unable to provide any language support. Most likely connected with an exception:
Describe the solution you'd like
Support all the standard language features. We would most likely also need to provide some notebook specific features (such as using values from other cells?).
Describe alternatives you've considered
None I can think of.
Additional contex
This came up in scalameta/metals#3213 and is separate to supporting https://code.visualstudio.com/api/extension-guides/notebook
Search terms
notebooks vscode
The text was updated successfully, but these errors were encountered: