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

isort --atomic fails on Cython "cimport" syntax #1722

Closed
DanielFEvans opened this issue May 6, 2021 · 5 comments
Closed

isort --atomic fails on Cython "cimport" syntax #1722

DanielFEvans opened this issue May 6, 2021 · 5 comments
Labels
enhancement New feature or request

Comments

@DanielFEvans
Copy link
Contributor

DanielFEvans commented May 6, 2021

isort --atomic fails for any Cython .pyx file containing a cimport statement, claiming it has an existing syntax error. A minimal example is:

cimport numpy
cimport ctime

isort buggy.pyx --diff runs fine:

$ isort buggy.pyx --diff
--- .../buggy.pyx:before       2021-05-06 10:19:09.495635
+++ .../buggy.pyx:after        2021-05-06 10:19:19.479452
@@ -1,2 +1,2 @@
+cimport ctime
 cimport numpy
-cimport ctime
Fixing .../buggy.pyx

However, isort buggy.pyx --diff --atomic fails:

$ isort buggy.pyx --diff --atomic
.../lib/python3.6/site-packages/isort/api.py:306: UserWarning: {file_path} unable to sort due to existing syntax errors

Bug found on isort v5.8.0, but the bug exists as far back as isort v5.0.0, when Cython support was added.

I presume the same issue will occur for any other Cython-specific syntax, too.

@timothycrosley timothycrosley added the enhancement New feature or request label May 7, 2021
@timothycrosley
Copy link
Member

Hi @DanielFEvans,

Sorry you experienced this error! I could see how it could be jarring! It is correct that atomic is incomptible with Cython, but that is simply because atomic is based fully on the ability to parse it after sorting with the CPython AST. I think expanding support to Cython as well would be awesome!

@DanielFEvans
Copy link
Contributor Author

Is it possible to enable atomic for normal Python and simultaneously disable it for Cython files? I've not spotted any configurability like that, but there are a lot of options!

@timothycrosley
Copy link
Member

It seems unlikely that Cython will in short order expose an AST parsing API so in the meantime, Ive improved the behavior to warn in verbose mode instead of fail when a Cython exentioned file contains a Python AST error. Hopefully, at some point in the future, Cython files will also be able to fully be checked for ast errors.

@JohnHBrock
Copy link
Contributor

With v5.9.1, I still see this error if I use the --check-only flag, e.g.:

$ isort --atomic --check-only buggy.pyx
ERROR: isort was told to sort imports within code that contains syntax errors: .../buggy.pyx.

Is this intentional?

@timothycrosley
Copy link
Member

@JohnHBrock no it is not intentional! I have pushed an change to fix this to the main branch, and plan to push a new release to PyPI sometime over the next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants