benlubas / molten-nvim

A neovim plugin for interactively running code with the jupyter kernel. Fork of magma-nvim with improvements in image rendering, performance, and more
GNU General Public License v3.0
608 stars 33 forks source link

[Bug] assertion error on :MoltenRestart #194

Open JJJHolscher opened 6 months ago

JJJHolscher commented 6 months ago
`:checkhealth molten` ![image](https://github.com/benlubas/molten-nvim/assets/25668596/3a6008a8-dfd4-4b0a-8867-7767dc1c85a2)
`:checkhealth provider` (the python parts) ![image](https://github.com/benlubas/molten-nvim/assets/25668596/f58779b1-4985-43d9-bc42-a131249b491b)

Description

After initializing, running any amount of cells and then calling :MoltenRestart neovim will show this after a second or 3: image The kernel does restart, though if I run a cell too quickly (before this error pops up, or shortly after) the cell outputs become misaligned. (cell #1 has no outputs, or only warnings and the output of cell #1 will be in that of cell #2, whose outputs are invisible (but will probably show up in cell #3 and cell #3's in cell #4's etc, though I didn't check that). This cell misalignment is not my main concern, I just want Molten to restart without neovim throwing an error.

Otherwise Molten works fine for me (some commands take a second or so, but that seems very normal). And note that MoltenRestart does restart the kernel, it just also throws an error.

Reproduction Steps

I've had this problem in multiple neovim setups. The first setup isn't available right now, but was written in lua without using any nix. The second setup is from nixvim. And my current configuration is pretty simple there:

plugins.molten.enable = true;

There are some other plugins active like dap, treesitter and a pyright language server, though I wouldn't think those should interfere here.

My python is installed through pyenv, I'm running ArchLinux x64.

I make a new kernel

python -m venv venv
. venv/bin/activate
pip install ipykernel
python -m ipykernel install --user --name python-vanilla
deactivate  # the error occurs regardless of any venv being active
mv ~/.ipython _ipython  # to be sure my ipython setup doesn't screw anyithng

Then I type

print("hello")

in a new neovim buffer

:MoltenEvaluateLine  # select the just-made python-vanilla kernel

The code runs : )

:MoltenRestart

The error shows most of the time. Sometimes it does not, if you don't get it on your first try, rerun the cell and restart again, I've never had to do more than 2 tries to get the error.

Note that I never explicitly make a python3 host, nixvim takes care of that for me. In my previous non-nix setup I would have a python3 host (that contained a decent amount of installed packages) which also would have this problem.

Expected Behavior

Restarts without errorring.

benlubas commented 6 months ago

Hmmm, I'm unable to reproduce this, but playing around with the restart command has made me realize that it doesn't behave sensibly in a few cases.

Personally I don't ever use it (it's just something that I brought over from magma). I can take a deeper look at how it's working and maybe figure out what's going on.

It would be helpful if you could provide a minimal config. There's an example in the docs folder for lazy, or if you prefer a nix flake of some kind, that would work too.

benlubas commented 6 months ago

You can also try this branch which has an added log statement, maybe the rest of the message will tell us what's happening, or maybe we can just ignore that case and it will be fine.

https://github.com/benlubas/molten-nvim/tree/fix/assert_fail_on_restart

JJJHolscher commented 6 months ago

I managed to reproduce the error with a minimal nixvim config:

flake.nix:

# from https://github.com/nix-community/nixvim/blob/main/templates/simple/flake.nix
{
  description = "A nixvim configuration";

  inputs = {
    nixpkgs.url = "github:nixos/nixpkgs/nixos-unstable";
    nixvim.url = "github:nix-community/nixvim";
    flake-parts.url = "github:hercules-ci/flake-parts";
  };

  outputs = {
    nixvim,
    flake-parts,
    ...
  } @ inputs:
    flake-parts.lib.mkFlake {inherit inputs;} {
      systems = [
        "x86_64-linux"
        "aarch64-linux"
        "x86_64-darwin"
        "aarch64-darwin"
      ];

      perSystem = {
        pkgs,
        system,
        ...
      }: let
        nixvimLib = nixvim.lib.${system};
        nixvim' = nixvim.legacyPackages.${system};
        nixvimModule = {
          inherit pkgs;
          module = import ./notebook.nix; # import the module directly
          # You can use `extraSpecialArgs` to pass additional arguments to your module files
          extraSpecialArgs = {
            # inherit (inputs) foo;
          };
        };
        nvim = nixvim'.makeNixvimWithModule nixvimModule;
      in {
        checks = {
          # Run `nix flake check .` to verify that your config is not broken
          default = nixvimLib.check.mkTestDerivationFromNixvimModule nixvimModule;
        };

        packages = {
          # Lets you run `nix run .` to start nixvim
          default = nvim;
        };
      };
    };
}

notebook.nix:

{ plugins.molten.enable = true; }

why I restart I need to restart since I do some debugging through my notebook. I debug some thing I'm importing by calling its functions in a notebook. importlib or the magic command do not always reimport the changes I made though, so I (unfortunately) need to restart Molten for that.

JJJHolscher commented 6 months ago

I also reproduced the error with lazy instead of nix

cd ~/.config/nvim
rm -r *
git clone https://github.com/benlubas/molten-nvim
cd molten-nvim
git switch fix/assert_fail_on_restart
cd ..
python -m venv venv
. venv/bin/activate
pip install pynvim jupyter_client cairosvg plotly kaleido pnglatex pyperclip
vim init.lua

init.lua:

vim.g.python3_host_prog=vim.fn.expand("~/.config/nvim/venv/bin/python")

local lazypath = vim.fn.stdpath("data") .. "/lazy/lazy.nvim"
if not (vim.uv or vim.loop).fs_stat(lazypath) then
  vim.fn.system({
    "git",
    "clone",
    "--filter=blob:none",
    "https://github.com/folke/lazy.nvim.git",
    "--branch=stable", -- latest stable release
    lazypath,
  })
end
vim.opt.rtp:prepend(lazypath)

require("lazy").setup({
  {
    dir = "~/.config/nvim/molten-nvim",
    build = ":UpdateRemotePlugins"
  }
})
nvim
print("hello world")
MoltenInit
MoltenEvaluateLine
MoltenRestart
MoltenRestart

The first :MoltenRestart seems to go fine, the second and any restarts after that error.

JJJHolscher commented 6 months ago

I noticed that I neglected to remove my standard neovim virtual environment located at ~/.config/nvim/.venv, which was activated in my tests.

After removing that and fixing a python_host_prog typo, the error message changed:

[Molten] Unexpected message status. Got `starting` when we didn't expect it. Here is the rest of the message:
{'exection_state': 'starting'}
Press ENTER or type command to continue
[Molten] Kernel 'python-vanilla' (id: python-vanilla) is ready.
Press ENTER or type command to continue

(which seems to be due to me using your new branch)

benlubas commented 6 months ago

Okay I got it to happen on the second restart now, let me play around with it a bit

Heh, yeah I got it once out of >20 tries. It's probably a race condition/performance based. Do you have a lot going on on your PC/laptop when it happens? Or do you have older hardware?

JJJHolscher commented 6 months ago

My specs are mediocre. I didn't notice a difference between my different devices, some of which are pretty bad and almost a decade old but the laptop I tried this on still sells for ~$700

benlubas commented 6 months ago

Does that new branch (which simply prints an error instead of asserting and crashing) allow the plugin to continue working even after you get the error?

If that's the case I might just remove the error entirely, just silently return when that happens.

JJJHolscher commented 6 months ago

That would work, but might leave one bug in molten

description if I MoltenEvaluate* too fast after a MoltenRestart (I think the timing is, I run the evaluation before the message pops up that the kernel is ready), Molten will misalign the output.

behavior The output cell of the first MoltenEvaluate* will keep showing Out[...]: * On Hold If I then MoltenEvaluate* on another code cell, the first output cell will change to what the second code cell outputs and the second output cell becomes Out[...]: * On Hold. If I then do a third MoltenEvaluate*, the pattern repeats (the second output cell receives the output from the third code cell again and the third output cell becomes Out[...]: * On Hold.)

screenshots After running 3 cells this way, the screenshots are like this: First cell: image Second cell: image Third cell: image

desired behavior It'd be nice if any MoltenEvaluate* commands are ignored or queued until the MoltenRestart finishes, but if that's too much work then I can live with this bug and just be patient about running evaluate commands.