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.

Sign up for GitHub

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

Jump to bottom

New feature: isort should move module-level-dunders to the correct place #1980

Open
clavedeluna opened this issue Oct 20, 2022 · 3 comments · May be fixed by #2060
Open

New feature: isort should move module-level-dunders to the correct place #1980

clavedeluna opened this issue Oct 20, 2022 · 3 comments · May be fixed by #2060

Comments

@clavedeluna
Copy link

clavedeluna commented Oct 20, 2022 •
edited
Loading

I can see in a related issue #711 dunders were at least prevented from moving. However, according to PEP8, dunders should come after future imports but before the other import block.

This is correct according to PEP8

from __future__ import barry_as_FLUFL

__all__ = ["barry_as_FLUFL"]
__version__ = '0.1'
__author__ = 'someone'

import os
import sys

This is incorrect


from __future__ import barry_as_FLUFL

import os 
import sys

__all__ = ["barry_as_FLUFL"]
__version__ = '0.1'
__author__ = 'someone'

But currently isort does not move the dunders. I propose this is implemented to correctly align with PEP8.
Related pylint issue pylint-dev/pylint#5471
@clavedeluna clavedeluna mentioned this issue Oct 20, 2022
Open
@kaparoo
Copy link

kaparoo commented Dec 11, 2022

Hi! Is there any further progress?

@clavedeluna clavedeluna linked a pull request Dec 22, 2022 that will close this issue
Open
@clavedeluna
Copy link
Author

In the course of working on this feature I've encountered an example which would defy the PEP8 instructions.

This example is actually within isort:

from importlib import metadata

__version__ = metadata.version("isort")

PEP8 says to move version up above the import, but it in fact depends on the import! I'm not sure what the guidance would be here, I can do some research, but I would expect not to change anything in this case. It would be a bit difficult to detect correctly though.

@clavedeluna clavedeluna mentioned this issue Jan 18, 2023
Closed
@clavedeluna
Copy link
Author

Given the resolution of python/peps#2971 (comment) I think there are two options here:

  1. If a dunder assignment relies on an import, that dunder assignment should be placed right after that import, separate from the other module dunders and potentially breaking up the import statements.
  2. Do not touch the code at all if any dunder assignment relies on an import line. This would at least not break anything and keep consistency.

I like 2 best for simplicity, but either option is going to require detecting if a dunder assignment relies on an import statement, which may be challenging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Isort orders module-level dunders clavedeluna/isort
2 participants
@kaparoo @clavedeluna