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

--sort-reexports is broken #2037

Closed
monosans opened this issue Dec 13, 2022 · 7 comments · Fixed by #2065
Closed

--sort-reexports is broken #2037

monosans opened this issue Dec 13, 2022 · 7 comments · Fixed by #2065
Labels
bug Something isn't working

Comments

@monosans
Copy link
Contributor

I decided to put two bugs in the same issue because I think sort-reexports is highly unstable and both bugs should be dealt with together.

Bug 1

__all__ = ('foo', 'bar')

Running isort --sort-reexports on this code creates a new __all__ instead of replacing the existing one.

__all__ = ('foo', 'bar')
__all__ = ('bar', 'foo')

Bug 2

If you make __all__ a tuple without parentheses, then isort will just crash.

__all__ = 'bar', 'foo'
ERROR: Unrecoverable exception thrown when parsing main.py! This should NEVER happen.
If encountered, please open an issue: https://github.com/PyCQA/isort/issues/new
Traceback (most recent call last):
  File "/home/user/.local/bin/isort", line 8, in <module>
    sys.exit(main())
  File "/home/user/.local/lib/python3.10/site-packages/isort/main.py", line 1225, in main
    for sort_attempt in attempt_iterator:
  File "/home/user/.local/lib/python3.10/site-packages/isort/main.py", line 1209, in <genexpr>
    sort_imports(  # type: ignore
  File "/home/user/.local/lib/python3.10/site-packages/isort/main.py", line 93, in sort_imports
    incorrectly_sorted = not api.sort_file(
  File "/home/user/.local/lib/python3.10/site-packages/isort/api.py", line 429, in sort_file
    changed = sort_stream(
  File "/home/user/.local/lib/python3.10/site-packages/isort/api.py", line 210, in sort_stream
    changed = core.process(
  File "/home/user/.local/lib/python3.10/site-packages/isort/core.py", line 236, in process
    code_sorting = LITERAL_TYPE_MAPPING[stripped_line.split(" = ")[1][0]]
KeyError: "'"

Technical info

OS: Arch Linux
Python 3.10.8
isort 5.11.1

@staticdev staticdev added the bug Something isn't working label Dec 13, 2022
@staticdev
Copy link
Collaborator

staticdev commented Dec 13, 2022

Indeed I reproduced the error, this option was introduced by parafoxia in this PR, maybe you have an idea what is wrong?

@parafoxia
Copy link
Contributor

parafoxia commented Jan 1, 2023

I actually did notice this, though am not 100% sure why it's happening. It wasn't happening when I developed it, but that was over a year ago so anything could've happened in the meantime. I'll have another look tomorrow probably. (Sorry, only just saw this. I actually ignored the error when I saw it recently cos I thought isort had been abandoned and it was never gonna see this feature, but now it's not I've actually gotta do something about it lmao.)

@parafoxia
Copy link
Contributor

parafoxia commented Jan 1, 2023

So I decided to look at it tonight, and both errors have been fixed @monosans @staticdev. I'm just gonna run through and double check stuff then create a PR.

@kevalmorabia97
Copy link

Seems like after the PR is merged, this still does not work as expected and breaks __all__ if --sort-reexports flag is used

@parafoxia
Copy link
Contributor

It's working fine for me. What version of isort are you using?

@mrcljx
Copy link

mrcljx commented Feb 2, 2023

@parafoxia Minimal example using 5.12.0:

from x import (
    y,
)


def z():
    return y()


__all__ = [
    "z",
    "y",
]

emits as

from x import y


def z():
   __all__ = ['y', 'z']
 = [

Seems to be triggered by the multi-line from x import (changing it to from x import y doesn't cause the issue).

@parafoxia
Copy link
Contributor

Ah, I think I see what might be happening here. I'll try and have a look at it some point over the next few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants