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

Can autoreload magics be kept above isort output? #488

Closed
Alexander-Serov opened this issue Nov 25, 2020 · 33 comments
Closed

Can autoreload magics be kept above isort output? #488

Alexander-Serov opened this issue Nov 25, 2020 · 33 comments

Comments

@Alexander-Serov
Copy link

Alexander-Serov commented Nov 25, 2020

I have tried to look around, but I am not sure if one of the recent fixed addresses this problem. I am using nbQA for its isort functionality and have the following problem.

A notebook which looks like this (MWE)

%load_ext autoreload
%autoreload 2

import sys
import pathlib

is converted by nbqa isort into

import pathlib
import sys

%load_ext autoreload
%autoreload 2

which actually invalidates the autoreload extension that must be loaded before any imports. I have tried to use # isort:skip or put it in a different cell, but to no avail.
Do you know if there's a fix for this?

Otherwise, thanks for the great hooks!

@MarcoGorelli
Copy link
Collaborator

MarcoGorelli commented Nov 25, 2020

Hi @Alexander-Serov

Thanks for your report!

I couldn't reproduce this: I tried pasting your input into a cell and got the following:

$ nbqa isort Untitled3.ipynb  --nbqa-diff
Cell 1
------
--- Untitled3.ipynb
+++ Untitled3.ipynb
@@ -1,5 +1,5 @@
 %load_ext autoreload
 %autoreload 2

+import pathlib
 import sys
-import pathlib

Could you please paste the output of nbqa --version? I'd expect that updating to the newest version (0.5.0, though we'll soon make a 0.5.1 release) should fix this

Otherwise, thanks for the great hooks!

That's very kind of you, thanks! Please do let us know of any other issues you run into and/or if you have any ideas for how to improve it

@Alexander-Serov
Copy link
Author

Hey Marco!

Thanks for the fast reply. Here is my output

$ nbqa isort example.ipynb --nbqa-diff
Cell 1
------
--- example.ipynb
+++ example.ipynb
@@ -1,2 +1,5 @@
+import pathlib
+import sys
+
 %load_ext autoreload
 %autoreload 2

Cell 2
------
--- example.ipynb
+++ example.ipynb
@@ -1,2 +0,0 @@
-import sys
-import pathlib

To apply these changes use `--nbqa-mutate` instead of `--nbqa-diff`
$ nbqa isort --version
nbqa 0.5.0

@MarcoGorelli
Copy link
Collaborator

ah, so you have

# cell 1
%load_ext autoreload
%autoreload 2

# cell 2
import sys
import pathlib

?

@girip11
Copy link
Contributor

girip11 commented Nov 25, 2020

Does isort has - - float-to-top setting present it's configuration?

@Alexander-Serov
Copy link
Author

And if I put

ah, so you have

# cell 1
%load_ext autoreload
%autoreload 2

# cell 2
import sys
import pathlib

?

I have just checked and it's the same behavior when I put them in the same cell.

$ nbqa isort example-2.ipynb --nbqa-diff
Cell 1
------
--- example-2.ipynb
+++ example-2.ipynb
@@ -1,5 +1,5 @@
+import pathlib
+import sys
+
 %load_ext autoreload
 %autoreload 2
-
-import sys
-import pathlib

To apply these changes use `--nbqa-mutate` instead of `--nbqa-diff`

@MarcoGorelli
Copy link
Collaborator

Do you have a setup.cfg or isort.ini (or some other configuration file which isort uses) in your directory, with some options set?

Because I can't reproduce this unless, as suggested by Girish, I use --float-to-top:

(base) mgorelli@m-gorelli02:~$ nbqa isort Untitled3.ipynb --nbqa-diff
(base) mgorelli@m-gorelli02:~$ nbqa isort Untitled3.ipynb --nbqa-diff --float-to-top
Cell 1
------
--- Untitled3.ipynb
+++ Untitled3.ipynb
@@ -1,2 +1,5 @@
+import pathlib
+import sys
+
 %load_ext autoreload
 %autoreload 2

Cell 2
------
--- Untitled3.ipynb
+++ Untitled3.ipynb
@@ -1,2 +0,0 @@
-import pathlib
-import sys

To apply these changes use `--nbqa-mutate` instead of `--nbqa-diff`

@Alexander-Serov
Copy link
Author

Does isort has - - float-to-top setting present it's configuration?

Yes, you are right. That solves it! Sorry for the trouble.

@MarcoGorelli
Copy link
Collaborator

Cool, thanks both 👍 - closing then

@Alexander-Serov
Copy link
Author

We are indeed using float_to_top = True, but I was just surprised that it changes cell sequence.
Is there any way to sort within the same cell or is nbqa always regrouping them in the first cell?

@Alexander-Serov
Copy link
Author

Alexander-Serov commented Nov 25, 2020

Because I see that if I add a new cell on top, which is markdown, it stays where it is.
Is there a way to just apply isort to individual cells?

@MarcoGorelli
Copy link
Collaborator

Is there a way to just apply isort to individual cells?

that should be the default behaviour - if you just run nbqa isort notebook.ipynb, then it should keep each cell's imports separated. Do you have an example of where this is not happening?

@girip11
Copy link
Contributor

girip11 commented Nov 25, 2020

The reason imports go to top of the file with that setting is because nbqa converts the contents of all the "code cells" to become statements of a python file in the order in which they appear in the notebook. Ipython magics also become valid python statements

@Alexander-Serov
Copy link
Author

Alexander-Serov commented Nov 25, 2020

I see, thank you guys. But is there a way to override the float_to_top from isort.cfg in nbqa isort call? I don't have the right to change isort.cfg in the project.

@Alexander-Serov
Copy link
Author

Oh, I think I've found it.
Adding --nbqa-ignore-cells % does the trick! Thanks guys!

@MarcoGorelli
Copy link
Collaborator

Adding --nbqa-ignore-cells % does the trick!

That's a bit of a hack though, as it won't sort the content of those cells. Could you show us the content of isort.cfg?

@girip11
Copy link
Contributor

girip11 commented Nov 25, 2020

Try passing --nbqa-config setup.cfg so that isort will not use isort.cfg as the config file

@Alexander-Serov
Copy link
Author

Alexander-Serov commented Nov 25, 2020

Sure, this is what our ITs have put in the project (and what I cannot change):

[settings]
multi_line_output=3
line_length = 88
include_trailing_comma=True
force_grid_wrap = 0
use_parentheses = True
float_to_top=true
ensure_newline_before_comments = True
profile=black
known_third_party=
    bs4
    colorama
    dateutil
    ...

I guess most settings are set just to be compatible with black for non-ipynb file.

@girip11
Copy link
Contributor

girip11 commented Nov 25, 2020

If your isort config is present in setup.cfg, then create a different file called my_isort.cfg and pass it via the - -nbqa-config flag

@Alexander-Serov
Copy link
Author

Alexander-Serov commented Nov 25, 2020

Try passing --nbqa-config setup.cfg so that isort will not use isort.cfg as the config file

I guess I have to create a setup.cfg apart then?

@Alexander-Serov
Copy link
Author

If your isort config is present in setup.cfg, then create a different file called my_isort.cfg and pass it via the - -nbqa-config flag

Got you. That should work!

@MarcoGorelli
Copy link
Collaborator

wait, passing a custom my_isort.cfg will only work if my_isort.cfg is a config file understood by isort.

It should be possible to overwrite it, will just try something and get back to you

@Alexander-Serov
Copy link
Author

wait, passing a custom my_isort.cfg will only work if my_isort.cfg is a config file understood by isort.

It should be possible to overwrite it, will just try something and get back to you

Yes, I was just wondering if I can tell nbqa isort to ignore configs altogether.

@MarcoGorelli
Copy link
Collaborator

MarcoGorelli commented Nov 25, 2020

Got it - you can just do

$ nbqa isort Untitled3.ipynb --nbqa-diff --float-to-top=0

EDIT: ignore this message, this soln is wrong

@girip11
Copy link
Contributor

girip11 commented Nov 25, 2020

wait, passing a custom my_isort.cfg will only work if my_isort.cfg is a config file understood by isort.

I thought a file like my_isort.cfg with the below contents should work

[isort]

@Alexander-Serov
Copy link
Author

Alexander-Serov commented Nov 25, 2020

Got it - you can just do

$ nbqa isort Untitled3.ipynb --nbqa-diff --float-to-top=0

Weirdly, it produces no output at all (so sys and pathlib are not sorted either). I have tried --float-to-top=false, but it doesn't work either. But this is exactly what I am looking for.

@Alexander-Serov
Copy link
Author

wait, passing a custom my_isort.cfg will only work if my_isort.cfg is a config file understood by isort.

I thought a file like my_isort.cfg with the below contents should work

[isort]

Yes, this could absolutely be a solution. I just need to create a file 👍

@MarcoGorelli
Copy link
Collaborator

MarcoGorelli commented Nov 25, 2020

you're right, --float-to-top=0 seems to disable isort. Maybe that's a question for the isort repo then - is there a way to override float_to_top from the command line?

And yup, if you just want to ignore the present configs, then passing some dummy .cfg file to --nbqa-config should do the trick, as suggested by Girish

@girip11
Copy link
Contributor

girip11 commented Nov 25, 2020

you're right, --float-to-top=0 seems to disable isort.

That's weird right. I thought setting to 0 would work

@Alexander-Serov
Copy link
Author

you're right, --float-to-top=0 seems to disable isort.

That's weird right. I thought setting to 0 would work

Yes, good point. But I have checked in the isort repo, and they only store true if the key is provided. So there seems to be no simple way to override.
https://github.com/PyCQA/isort/blob/2ba6bb4dd748f9d9300c797f5254bb742e805a30/isort/main.py#L422
I should ask them.

@Alexander-Serov
Copy link
Author

you're right, --float-to-top=0 seems to disable isort. Maybe that's a question for the isort repo then - is there a way to override float_to_top from the command line?

And yup, if you just want to ignore the present configs, then passing some dummy .cfg file to --nbqa-config should do the trick, as suggested by Girish

Thanks. Indeed, float-to-top is considered by isort guys as something that you would not supply at each run. So they seem to have not thought about a mechanism to disable it.

@Alexander-Serov
Copy link
Author

Thanks anyway for the discussion! Very helpful!

@Alexander-Serov
Copy link
Author

Btw, I have found another workaround: I can put the jupyter magic in the startup script of jupyter thus removing it from the nb.
It will apply to all notebooks, but for me it is the desired behavior.

@MarcoGorelli
Copy link
Collaborator

This'll be in isort 5.7.0 🎉 PyCQA/isort#1603

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants