Strange docs in "Known Limitations"

[sobolevn]

Hi!

I was reading your docs and then I found this:

The text says "after trailing comma", but the examples are using trailing ;
Which one is correct?

[MarcoGorelli]

It should be semicolon, thanks for catching this!

[MarcoGorelli]

I'm a fan of your blog btw (which I found thanks to an interview you gave, I think with podcast.init), honored to find you here

[sobolevn]

@MarcoGorelli oh, wow! Thanks a lot for your kind words and this project 😊
It is amazing! I am going to propose to use it with our https://github.com/wemake-services/wemake-python-styleguide

I invite you to contribute! wemake-services/wemake-python-styleguide#1704

[MarcoGorelli]

Nice! So the idea would be to use this to adapt your current highly opinionated style-guide to Jupyter Notebooks? If so, yeah, I'd be happy to contribute / work on this together!

It is amazing!

Thanks! Is it OK if I include this as a testimonial in the readme and link to your blog?

[sobolevn]

So the idea would be to use this to adapt your current highly opinionated style-guide to Jupyter Notebooks?

Yes, exaclty! The problem I trying to solve right now is that a lot of people use Python in Jupyter as their main env.
And this code 100% needs some love from a strict linter. And I have a lot of requests from users about how one can adopt our project. At the moment, there's no "official" way to do that, because, well, I don't use Jupyter that much myself.

But, that's where your project shines! It allows to use not just flake8, but all other tools as well: black, mypy, etc.

I would also love to hear users' insights about what rules are not applicable to Jupyter. Because I suspect there's some specific.

Any help is appreciated!

Is it OK if I include this as a testimonial in the readme and link to your blog?

I would be honoured! 🙇‍♂️

[MarcoGorelli]

I'd like to think that if you have flake8 and wemake-python-styleguide installed, then

nbqa flake8 my_notebook.ipynb --extend-ignore=E703

would work (though there are almost certainly many more extra error codes we would need to ignore).

We would also need to get #377 merged so we catch cases when code is hidden inside magic commands, e.g. we wouldn't want

import pandas as pd

%time pd.DataFrame()

to return "pandas as pd imported but unused". But that PR should cover such cases.

In terms of implementation - if we added tests specifically for wemake-python-styleguide and made a nbqa-wemake-python-styleguide pre-commit hook, would that work for you, or did you want this to live inside the wemake-python-styleguide repo?

[sobolevn]

In terms of implementation - if we added tests specifically for wemake-python-styleguide and made a nbqa-wemake-python-styleguide pre-commit hook, would that work for you, or did you want this to live inside the wemake-python-styleguide repo?

I am not sure. Do you have an experience with similar tests? Which one works best?

[MarcoGorelli]

Yes - the way nbqa works is that it converts the notebooks you pass it to temporary Python files (with some adjustments for magics, such as

%%time

which aren't valid Python), runs the code quality tool on them, and then parses the output. Optionally (though this wouldn't be the case for wemake-python-styleguide) it reconstructs your notebook with the formatting having been applied. It's meant to be tool-agnostic (though we may occasionally have to do some special casing for some tools).

For the tools we "officially" support (black, flake8, mypy, pyupgrade, isort, doctest, pylint) we have tests checking that the output looks the way it's meant to. As wemake-python-styleguide is a flake8 plugin, it should just be a matter of extending our tests, but it should already work out of the box.

Simply, instead of

flake8 your_module.py

you would run

nbqa flake8 your_notebook.ipynb

It would be useful if you could try running that on some notebooks to see if the output is what you would expect (and report back if it isn't) - barring the aforementioned cases of inline magics where you might get some false-positives, my expectation is that this should work the way you expect it to

[MarcoGorelli]

my expectation is that this should work the way you expect it to

Nevermind, I just tried this on a notebook and got

$ nbqa flake8 comprehensive-data-exploration-with-python.ipynb 
Traceback (most recent call last):
  File "/home/marco/miniconda/bin/nbqa", line 8, in <module>
    sys.exit(main())
  File "/home/marco/miniconda/lib/python3.7/site-packages/nbqa/__main__.py", line 595, in main
    output_code = _run_on_one_root_dir(cli_args, configs, project_root)
  File "/home/marco/miniconda/lib/python3.7/site-packages/nbqa/__main__.py", line 528, in _run_on_one_root_dir
    out, notebook, nb_info_mapping[notebook].cell_mappings
  File "/home/marco/miniconda/lib/python3.7/site-packages/nbqa/__main__.py", line 126, in _map_python_line_to_nb_lines
    out = re.sub(pattern, substitution, out)
  File "/home/marco/miniconda/lib/python3.7/re.py", line 192, in sub
    return _compile(pattern, flags).sub(repl, string, count)
  File "/home/marco/miniconda/lib/python3.7/site-packages/nbqa/__main__.py", line 124, in substitution
    return str(cell_mapping[int(match.group())])
KeyError: 0

So, we have some work to do with parsing the output from the extra plugins before we can claim to support wemake-python-styleguide - I'll get back to you on this anyway!

[MarcoGorelli]

@all-contributors please add @sobolevn for ideas, bugs, doc

[allcontributors]

@MarcoGorelli

I've put up a pull request to add @sobolevn! 🎉

[s-weigand]

@sobolevn Until nbqa has support for WPS, you can check if flake8-nb works for you.
Since it is a hack on flake8 internals, it shouldn't have problems with multiline error codes and work out of the box.

@MarcoGorelli sorry for promoting my own tool here 😞

[MarcoGorelli]

@s-weigand no need to apologise - this is open source, we all share the benefits of each others' work 😄 If anything, thanks for all your amazing contributions here! I was going to try flake8-nb with WPS out to see if it worked, looks like you beat me to it 😄

For completeness, flake8-nb also has the limitation which we're trying to get around:

Untitled.ipynb:

import numpy as np

%time np.random.randn(1000)

command-line:

$ flake8-nb Untitled.ipynb 
Untitled.ipynb#In[ ]:6:1: WPS102 Found incorrect module name pattern
Untitled.ipynb#In[ ]:5:1: D100 Missing docstring in public module
Untitled.ipynb#In[ ]:4:1: I004 isort found an unexpected blank line in imports
Untitled.ipynb#In[ ]:1:1: F401 'numpy as np' imported but unused
Untitled.ipynb#In[ ]:1:1: I001 isort found an import in the wrong position

But still, I think such cases are rare and so it's probably a good way of running WPS for >99% of cases

[MarcoGorelli]

my expectation is that this should work the way you expect it to

Nevermind, I just tried this on a notebook and got

$ nbqa flake8 comprehensive-data-exploration-with-python.ipynb 
Traceback (most recent call last):
  File "/home/marco/miniconda/bin/nbqa", line 8, in <module>
    sys.exit(main())
  File "/home/marco/miniconda/lib/python3.7/site-packages/nbqa/__main__.py", line 595, in main
    output_code = _run_on_one_root_dir(cli_args, configs, project_root)
  File "/home/marco/miniconda/lib/python3.7/site-packages/nbqa/__main__.py", line 528, in _run_on_one_root_dir
    out, notebook, nb_info_mapping[notebook].cell_mappings
  File "/home/marco/miniconda/lib/python3.7/site-packages/nbqa/__main__.py", line 126, in _map_python_line_to_nb_lines
    out = re.sub(pattern, substitution, out)
  File "/home/marco/miniconda/lib/python3.7/re.py", line 192, in sub
    return _compile(pattern, flags).sub(repl, string, count)
  File "/home/marco/miniconda/lib/python3.7/site-packages/nbqa/__main__.py", line 124, in substitution
    return str(cell_mapping[int(match.group())])
KeyError: 0

So, we have some work to do with parsing the output from the extra plugins before we can claim to support wemake-python-styleguide - I'll get back to you on this anyway!

The latest release (0.3.6) solves this problem, which was due to WPS giving warnings about line 0 (which I hadn't planned for 😳)

There's still some work to do before the in-line magics limitation is solved, but...we're getting there 🚀