blacken-docs icon indicating copy to clipboard operation
blacken-docs copied to clipboard

Add support for notebook style code

Open znicholls opened this issue 1 year ago • 5 comments

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 https://github.com/adamchainz/blacken-docs/pull/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 https://github.com/adamchainz/blacken-docs/pull/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 avatar Apr 24 '23 08:04 znicholls

@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?

znicholls avatar May 01 '23 10:05 znicholls

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?

adamchainz avatar Aug 16 '23 07:08 adamchainz

I previously discussed a "check"/"dry run" feature in https://github.com/adamchainz/blacken-docs/issues/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 avatar Aug 17 '23 15:08 znicholls

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)

znicholls avatar Aug 17 '23 19:08 znicholls

Hi @adamchainz, I'm back looking at stuff related to this again and wondered if you had any thoughts? I'll then add them to #239 and mark that as ready for review. Thanks again

znicholls avatar Aug 12 '24 13:08 znicholls