Skip to content
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

I have a bunch of contributions to make #20

Open
benlubas opened this issue Dec 22, 2024 · 3 comments
Open

I have a bunch of contributions to make #20

benlubas opened this issue Dec 22, 2024 · 3 comments

Comments

@benlubas
Copy link

@AbaoFromCUG I'm writing this issue to let you know that I have a few features that I'd like to contribute. I've already implemented everything I'm about to describe.

I'll do my best to split these features into different PRs, but inevitably some changes are going to stack on top of each other. Here's a list of what to expect in the next few days/weeks:

  1. I switched the logger to a better one (Neorg, Plennary, and many other plugins use a similar single file logger and I quite like it)
  2. Supporting any language in percent style notebooks by using 'commentstring' instead of hard coded #
  3. Modular parsing system, configurable parser based on file globs
  4. Support for markdown and spin style plaintext notebooks (enabled by the modular parsing system)
  5. what I'll call "temp file mode", instead of having to keep a plaintext and ipynb file around, you can choose to have neopyter create and delete temporary ipynb files when you create and delete a buffer in neovim. This is the largest change in this list, and it changes the way that you use this plugin a lot. The way that I've implemented it, you can still use the plugin in the old way, you can choose one or the other, but honestly I stopped testing things in the normal mode, I only use temp file mode. So this change will require some testing probably
  6. :Neopyter console command that opens a split console to the same kernel that's running in Jupyter Lab. This uses jupyter-kernel under the hood, tmux and nvim terminal are supported. In the case that tmux is used, setup the appropriate buffer local variables to allow vim-slime to just work
  7. removes the requirement to open nvim and JL with the same base directory by converting everything to absolute file paths
  8. Parse metadata from plaintext files so that new files are created with the kernel given in the metadata

Along with these changes comes a large documentation overhaul, and I've touched nearly every file in the codebase. Sorry I'm dumping all of this on you at once, if I could have, I would have contributed as this stuff was implemented.

Is there anything in this list you don't want me to contribute?

I will probably wait to split things up until you've commented. I will put up a branch shortly that has all of these changes rolled together if you're curious.

Also as you might imagine, this stuff breaks a lot of tests. I didn't write any tests myself. But I can add them before opening PRs, it will just slow down the rate of PRs a good bit

@benlubas
Copy link
Author

Here's the diff:
master...benlubas:neopyter:push-kntnkvvnurut

@AbaoFromCUG
Copy link
Collaborator

We warmly welcome your contribution. Here is some comment/context I want to share with you.

  1. logger can be completely replaced, and can be output to files, under the vim.fn.stdpath("cache") directory.
  2. Regarding to percent format, we has refenence the specification of jupytext. This may be a good start, the principle is universal/concise. Do what you want to do.
  3. Parse system should rewrite to treesitter based (current is regex-based), but I worry about the performance of treesitter
  4. Same as comment 3, implement via query of treesitter
  5. "temp file mode" is great, the implment could reference jupytext, see how-to-open-a-text-notebook-in-jupyter-lab
  6. :Neopyter console is welcome, the code could run with commands.execute('console:inject', { activate, code, path }) in frontend. I could provide assistance in this part.
  7. Could you check filename_mapper config? do you means that we should change the parameters of filename_mapper to absolute path? ( indeed to be done this way but not). But I think to sync /abs/path/to/main.ju.py with /another/abs/path/to/main.ipynb is not a design. could you share more details?
  8. metadata parser is import. LGTM

Most of test code is utility test or parser test, I could like to remove some test for a good start for you.

Thank you very much for your comment. You can open a PR at any time

@AbaoFromCUG
Copy link
Collaborator

Hi, I had cleared the code:

  • Remove blocking test case
  • Optimize per-commit check
  • Upgrade/simplify the eslint config

I have to admit, this project is very complex. Some helpful tips:

Dev setup

Ref to docs

  • Dependencies
pip install hatch hatch-nodejs-version pytest pytest_jupyter ruff
npm install -g pnpm
  • Setup
git clone https://github.com/SUSTech-data/neopyter
cd neopyter
pnpm install
# which will install additional tool for dev mode
pip install -e ".[dev]"
  • Dev watch mode
  1. In one terminal, pnpm watch
  2. In another terminal, jupyter lab

in this mode, you can modify the .ts file, then watch the effect after refresh browser

Neopyter async context

  • Neopyter use plenary.async as an async library. Most neovim callback (e.g. autocmd) is not in async context, you need wrap with a.run in this case.

If you have any questions, feel free to comment at any time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants