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

Add support for notebook style code #240

Open
znicholls opened this issue Apr 24, 2023 · 4 comments
Open

Add support for notebook style code #240

znicholls opened this issue Apr 24, 2023 · 4 comments

Comments

@znicholls
Copy link

Description

Firstly thanks for such an awesome tool.

In my repository, I wrote jupyter notebooks, which I then save to markdown files using jupytext. I would like to run black over these files. blacken-docs is almost perfect for this, but it just needs support for this style of code block.

I've put an implementation here #239, I needed to do two things:

  1. add a parser for this style of code block
  2. handle the fact the code blocks come out of jupyter notebooks (which fortunately is simple because black can already handle these)

As part of #239, I also added a 'check' functionality.

It would be great to get feedback on:

  1. whether adding this new parser is something you would support (I'm happy to maintain it)
  2. whether the 'check' functionality is of interest

Assuming that the answer to either or both of the above is yes, I will make a new PR for them (also separating them so that the two changes don't end up being coupled).

This is sort of related to #127 I think

Thanks again for your efforts!

@znicholls
Copy link
Author

@adamchainz I don't want to be too pushy, but as you are the maintainer would I be able to ask if you have any thoughts on this?

@adamchainz
Copy link
Owner

Yes, let's add the new parser, but leave the "check" function for now.

I previously discussed a "check"/"dry run" feature in #197 and concluded that it's not worth it when git restore can undo changes.

Please can you update your PR to include docs, tests, and a changelog note?

@znicholls
Copy link
Author

znicholls commented Aug 17, 2023

I previously discussed a "check"/"dry run" feature in #197 and concluded that it's not worth it when git restore can undo changes.

Cool will do.

Please can you update your PR to include docs, tests, and a changelog note?

Can do. Re the tests, if I run them currently I am getting failures locally (running just using e.g. tox run -f py39). Is that also the current state in CI? Ignore me

The other thing I seem to need to add is tokenize-rt. I saw the files in requirements. Is that your preferred way to add a dependency (in which case, how is it best to recompile the files, is there some trick to setting up all the right environments?) or is it better to use setup.cfg for this? Also ignore this for now, not clear to me yet whether I actually need it or not.

znicholls added a commit to znicholls/blacken-docs that referenced this issue Aug 17, 2023
@znicholls
Copy link
Author

MyST is designed with notebook support in mind (amongst other things). As a result, you have to use black's format_cell function. format_cell depends on tokenize-rt and (sometimes) IPython being installed.

I don't think these need to be added as dependencies because the module not found error is clear enough for the user (but I'm happy if you have a way you want to handle optional dependencies). Ways I could see to handle this:

  • Make tokenize-rt and IPython required dependencies of blacken-docs (feels wrong to me as I said above)
  • Make tokenize-rt and IPython optional dependencies of blacken-docs
  • Just add a note in the README that you may need extra dependencies if you want to format notebook style cells (but otherwise leave it to users)

For the tests, these extra modules do need to be installed for the MyST-related tests to pass. What is your preferred way to do this? Options I see:

  • Add to the files in requirements. If this way, how is it best to recompile the files, is there some trick to setting up all the right environments?
  • Make tokenize-rt and IPython required dependencies of blacken-docs (feels wrong to me as I said above)
  • Create a new tox environment just for testing the notebook-related stuff (marking all the notebook tests so that they skip if the notebook requirements aren't installed)

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