nbQA-dev / nbQA

Run ruff, isort, pyupgrade, mypy, pylint, flake8, and more on Jupyter Notebooks
https://nbqa.readthedocs.io/en/latest/index.html
MIT License
1.02k stars 39 forks source link

False positive for `ruff` `I001` (isort) when there is a function/class definition after the imports cell #806

Closed felix-cw closed 1 year ago

felix-cw commented 1 year ago

This bug is similar to #796 (thank you very much for fixing that!). It arises when running nbqa ruff

As in #796, there is a false positive error message of

I001 [*] Import block is un-sorted or un-formatted

and --fix does not change the notebook, even though it reports it as fixed.

My minimal notebook has the following cells:

import os
class A:
    ...

If I add the --diff option, it wants to insert a newline

@@ -1,6 +1,7 @@
 # %%NBQA-CELL-SEPfc4e93
 import os

+
 # %%NBQA-CELL-SEPfc4e93
 class A:
     ...

Would fix 1 error.
MarcoGorelli commented 1 year ago

thanks @felix-cw for the report, will take a look

MarcoGorelli commented 1 year ago

Right, so ruff wants two newlines before a definition or a class, and one newline otherwise, it seems:

# %%NBQA-CELL-SEPb4b132
import os

# %%NBQA-CELL-SEPb4b132
cwd = os.getcwd()
x = np.arange(1, 10)

# %%NBQA-CELL-SEPb4b132
import foo

# %%NBQA-CELL-SEPb4b132
class Foo:
    ...

# %%NBQA-CELL-SEPb4b132
import foo

# %%NBQA-CELL-SEPb4b132
def foo():
    ...

The following passes

I may need to preprocess the temporary Python file before passing it to ruff...

MarcoGorelli commented 1 year ago

one solution could be to pass autopep8 with only codes E3 selected first

this'd be a pretty big change, and I'll need to test it out thoroughly first. it might even warrant a 2.0 release. but it might be the right thing to do

MarcoGorelli commented 1 year ago

@felix-cw this should be solved in version 1.7.0, would appreciate it if you could check - thanks 🙏

felix-cw commented 1 year ago

@MarcoGorelli Thanks so much for working on this. I'll make sure to have a look later on and let you know on here.

felix-cw commented 1 year ago

I hope you don't mind, but I use conda-forge and the autotick bot seems like it's not working. I took the liberty of opening a PR on the feedstock repo to update.

MarcoGorelli commented 1 year ago

of course! @all-contributors please add @felix-cw for infra

allcontributors[bot] commented 1 year ago

@MarcoGorelli

I've put up a pull request to add @felix-cw! :tada: