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

nbQA mistakenly reports formatting symbol as invalid syntax because it detects it as IPython magic #594

Closed
MarcoGorelli opened this issue May 20, 2021 · 3 comments · Fixed by #595

Comments

@MarcoGorelli
Copy link
Collaborator

xref yt-project/yt#3287 :

(Pdb) !line
'    % hot_ad["temperature"]\n'
(Pdb) IPythonInputSplitter().transform_cell(line)
'get_ipython().run_line_magic(\'hot_ad\', \'["temperature"]\')\n'

I'm tempted to just stop trying to handle inline magic, it's too complicated and unreliable and they don't come up particularly often anyway. Will try to get something together at the weekend

@s-weigand
Copy link
Contributor

Not handling inline magic might also lead to problems, remember this code we didn't know was valid before someone opened an issue to support it? 😅

nbQA/nbqa/handle_magics.py

Lines 292 to 295 in 7725b79

import os
if True:
%time os.system("ls -l")

Not sure about how much of a performance impact it would be, but a possibly more robust way to check if a cell contains magic might be to call IPythonInputSplitter().transform_cell on the whole cell, check if the source changed and only handle it differently if it does.

Btw. I guess this is also the reason why pyupgrade isn't replacing this old-style percent formatting.

@MarcoGorelli
Copy link
Collaborator Author

I'm pretty tempted to just ignore any cell which contains line magics and just treat that as a known limitation

@MarcoGorelli
Copy link
Collaborator Author

I'm pretty tempted to just ignore any cell which contains line magics and just treat that as a known limitation

I think I was being overly negative when I wrote that - in #595, we can handle cells with line magics and handle this bug. It does mean giving up on checking Python code within a line magic command, but that's a limitation I'm happy to live with for now

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