-
Notifications
You must be signed in to change notification settings - Fork 386
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
Black interferes with py:percent format in indented code blocks. #562
Comments
Hi @Skylion007 , that is an interesting report! Do we agree that already, running I see that issues about comment indentation have been reported (and fixed) at psf/black#16 and psf/black#262, but I think here we are in a different case. For instance the cell-marker comment Also, I have another question for you: with that code in the Python script, what do you expect to get in Jupyter? AFAIK you cannot break down the |
@mwouts You actually can break down an if statement instruction into multiple code cells and still have it be runnable! Try it, it works in Google Collab at the very least. It's also runnable directly in VSCode. I've edited the minimal example to more closely mirror my actual code. In the updated code, the first cell should print "1" and the 2nd cell should print "2". |
@mwouts I have a workaround for it now, but it's not very elegant. Also, another issue to raise is that's impossible to escape spaces within a command in --pipe which created a lot of trouble writing the proper sed expression and having it run:
|
Hi @Skylion007 , thanks for the details, I didn't know about these partial cells, that's interesting! Do you think that allowing indented |
It would help absolutely, but I am not sure if it's part of the 'standard' per say so it should probably be behind a flag/option maybe? After all, the py:percent format isn't JuPyText specific, not sure if Spyder or other IDEs respect the indented comments. Also, this could be an issue with round trip conversions with other formats, no? What about indented doc strings as text markers etc... Also, doing it creates some ambiguity of what the 'right' indentation should be. Like if I move the comment further right along with the code block, does all the code respect it's indentation or not? That could be another way you could 'guard' code in a main method and have it be the same between a notebook and a script. |
Well, my assumption being that most code cells start with non-indented code, this would concern very few notebooks/cells, so maybe making this optional is not really required. But sure, if you think otherwise, or if we find any occurrence in representative repos like e.g. Python for Data Science, we can make this optional.
I agree. Still, maybe that is not a blocker? In the worst case, having an indented cell marker for the second cell in your example will just mean that some IDE will run the two cells as just one. Maybe the reasons for doing this, which are 1) with the indented cell marker, the jupytext file looks more Pythonic, and 2) it works better with
Here we are only discussing the indentation for the |
That's a fair argument, actually it probably should be the default. I just tested it and VSCode respects indented comments at the very least so that make me feel better it. The concern is with point 2 though as all my break points are under my if statement so blurring all the cells together make them function as one giant cell. As for the 'right' indentation, I think you are right as that is a pretty odd an edge case if that's not the case. I can't think of any practical use case where you would want to change the indentation of a cell unless you want to do it for the entire script. TLDR: On second thought I agree, make it a default option and that should hopefully solve this issue for the Py:Percent format at least. |
I found no indented code cell in https://github.com/jakevdp/PythonDataScienceHandbook: git clone https://github.com/jakevdp/PythonDataScienceHandbook.git
cd PythonDataScienceHandbook import os
import nbformat
for nb_file in sorted(os.listdir('notebooks')):
if not nb_file.endswith('.ipynb'):
continue
nb = nbformat.read(os.path.join('notebooks', nb_file), as_version=4)
for cell in nb.cells:
if cell.cell_type!="code":
continue
for line in cell.source.splitlines():
# is the first non-empty line indented?
if line.strip():
if line.startswith(' '):
print(cell.source)
break so changing the indent of the |
Found another interesting edge case to keep in mind. The following cell works when transferred to colab.
but the following does not:
The extra newline at the beginning of the cell seems to reset the indentation of the entire cell. Just something to keep in mind. JuPyTer and VSCode however don't seem to care and both cells work fine in both of them. Colab gives an indentation error though. |
Hi @Skylion007 , would you mind giving it a try to the branch indented_markers_percent? I think you can simply do
|
@mwouts Yeah, that seems to fix it on my end. At least presuming it rounds trips with --test locally and the outputted notebook appears to be identical. Here is the example colab btw: https://github.com/facebookresearch/habitat-sim/blob/master/examples/tutorials/colabs/rigid_object_tutorial.ipynb |
Sorry, it took me weeks to reproduce this one... the good news is that I just hit the same issue when working on #553. It should be fixed on the branch |
I am having an issue where py:percent notebooks that have cells divided within an if statement get destroyed by black when using --pipe black
gets reformatted into the following, which is not equivalent
For example, when running the following.
jupytext --to notebook test.py
jupytext --to py:percent --pipe black test.ipynb
Will cause all cells that contain only indented code to be merged into one giant cell. Are they any formats / workarounds to ensure black does not destroy the scripts like so? Or are there any relevant settings that jupytext can set that will resolve this issue?
The text was updated successfully, but these errors were encountered: