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

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

Closed
felix-cw opened this issue Mar 28, 2023 · 8 comments · Fixed by #807

Comments

@felix-cw
Copy link

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

  • on a notebook with any import in the first cell
  • where the isort checks are enabled e.g. with --select=I
  • The cell following the cell containing imports starts with a class or function definition

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
Copy link
Collaborator

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

@MarcoGorelli
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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

@felix-cw
Copy link
Author

felix-cw commented Apr 3, 2023

@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
Copy link
Author

felix-cw commented Apr 3, 2023

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
Copy link
Collaborator

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

@allcontributors
Copy link
Contributor

@MarcoGorelli

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

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

Successfully merging a pull request may close this issue.

2 participants