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

System assigns result in invalid syntax #816

Open
MarcoGorelli opened this issue Jul 12, 2021 · 5 comments
Open

System assigns result in invalid syntax #816

MarcoGorelli opened this issue Jul 12, 2021 · 5 comments

Comments

@MarcoGorelli
Copy link

If I have a cell with

flake8_version = !flake8 --version
flake8_version

, then this executes fine.

However, the equivalent Python representation of it from jupytext is:

# %%
flake8_version = !flake8 --version
flake8_version

which is invalid syntax (and not something I could run flake8 on 😄 )

I'm bringing this up because in #781 you commented

magic commands are always commented out by default

but this one wasn't commented out.

From a quick glance at the source code, it looks like magics are detected using regular expressions - have you considered using IPython's TransformerManager and then ast-parsing instead? I'm happy to help out with this if you want greater support for magic detection (at least, in Python notebooks)

@MarcoGorelli
Copy link
Author

Likewise for

time = %time 2+2

% jupytext --version
1.11.3

@mwouts
Copy link
Owner

mwouts commented Jul 12, 2021

Hi @MarcoGorelli , oh that's right there are so many magic commands... Is it correct that we have implemented support for var = %magic command but not for var = !bash command?

Thanks also for pointing out to TransformerManager, indeed your other comment is helpful, and could certainly provide a path to a Python representation that is fully executable.

We can think a bit more about that. On the one hand, I must admit that the support for magic commands is error prone (the fanciest error being the one where variables named cat were commented out at #339), on the other hand I like the idea that the script looks like the notebook, and that the 'uncomment' operation is simple.

Also, maybe changing this for everyone would cause many diffs on existing notebooks. So maybe, if we do implement a version of the magic commands based on the TransformerManager, we should make it optionally?

@MarcoGorelli
Copy link
Author

Also, maybe changing this for everyone would cause many diffs on existing notebooks.

Agreed - but you can keep the commented out magics using this approach. You just use TransformerManager to detect where the magics are, then you "extract" the magic using ast.parse, then you comment / uncomment - so the script would still look like the notebook

In psf/black#2357 I replace the magics with randomly generated tokens, but they could just as easily be replaced with commented-out versions of themselves

I'll put something together to demonstrate when I get a chance

@mwouts
Copy link
Owner

mwouts commented Jul 14, 2021

I'll put something together to demonstrate when I get a chance

Thanks, that would be great! Alternatively if you have some documentation on the TransformerManager I can take a look at it.
Also I like the idea to keep provinding the simple commented form, but I think the over version were the script is completely executable might also be of interest for people wanting to run the notebook as a script... we'll see how that goes!

@MarcoGorelli
Copy link
Author

Other than the official does, I don't have any extra documentation (other than the docstrings in src/black/handle_ipynb_magics.py from https://github.com/psf/black/pull/2357/files )

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

No branches or pull requests

2 participants