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.04k stars 40 forks source link

Strange behaviour when applying nbqa black to commented ipython magic #495

Closed john-sandall closed 3 years ago

john-sandall commented 3 years ago

Behavior

Running nbqa black on this file shifts cells' contents into the next cell down. Seems to be related to the presence of # %%capture.

Steps to Reproduce

  1. Run nbqa black '1 - Minimal nbqa black breaking example 1 - goes nuts.ipynb' on the file linked above.
  2. Notice that the contents of cell 5 are now in cell 6, with the commented ipython magic cell # %%capture now uncommented with the %% removed. The contents of cell 6 are now in cell 7, but the output of cell 7 hasn't changed.
  3. We noticed that this only happens if a) there's a cell in the notebook that that black needs to reformat (hence the cell at the top), and b) a commented out # %%capture is present.
  4. Removing either the top cell (so there's no black formatting applied) or uncommenting the capture magic so it's just %%capture runs as expected with no funky cell switcheroo.

Environment

Mitigations

We were using %%capture to inhibit output from matplotlib. We now call plt.close() within our plotting code instead.

MarcoGorelli commented 3 years ago

Hey @john-sandall , good to hear from you, and thanks for the excellent bug report!

We use # %% to separate the cells when converting the notebook to a temporary Python file, so it looks like something's going wrong when reconstructing the notebook - nonetheless, commented-out magics shouldn't break nbqa, so I've labelled this as a bug

Will look into it!

MarcoGorelli commented 3 years ago

@all-contributors please add @john-sandall for bugs

allcontributors[bot] commented 3 years ago

@MarcoGorelli

I've put up a pull request to add @john-sandall! :tada:

MarcoGorelli commented 3 years ago

Just to confirm - you've only noticed this when the magic (in this case, %% capture) was commented out, right? Uncommenting the magic so we have

In [1]: [1, 2,
   ...: 3, 4]

In [2]: %%capture
   ...: 
   ...: p1 = 1
   ...: print(p1)

works fine for me:

$ nbqa black johns_mcve.ipynb --nbqa-diff
Cell 1
------
--- johns_mcve.ipynb
+++ johns_mcve.ipynb
@@ -1,2 +1 @@
-[1, 2,
-3, 4]
+[1, 2, 3, 4]

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

while commenting out the magic so we have:

In [1]: [1, 2,
   ...: 3, 4]
Out[1]: [1, 2, 3, 4]

In [2]: # %%capture
   ...: 
   ...: p1 = 1
   ...: print(p1)

returns the nonsense

$ nbqa black johns_mcve.ipynb --nbqa-diff
Cell 1
------
--- johns_mcve.ipynb
+++ johns_mcve.ipynb
@@ -1,2 +1 @@
-[1, 2,
-3, 4]
+[1, 2, 3, 4]

Cell 2
------
--- johns_mcve.ipynb
+++ johns_mcve.ipynb
@@ -1,4 +0,0 @@
-# %%capture
-
-p1 = 1
-print(p1)

Cell 3
------
--- johns_mcve.ipynb
+++ johns_mcve.ipynb
@@ -0,0 +1,4 @@
+capture
+
+p1 = 1
+print(p1)

To apply these changes use `--nbqa-mutate` instead of `--nbqa-diff`
MarcoGorelli commented 3 years ago

@john-sandall this should be fixed in version 0.5.3, if you could update and check we'd be really grateful. And please do let us know if you run into any further issues :pray: