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

Add option to move module level dunders to top of file #9324

Open
nstarman opened this issue Dec 30, 2023 · 4 comments
Open

Add option to move module level dunders to top of file #9324

nstarman opened this issue Dec 30, 2023 · 4 comments
Labels
rule Implementing or modifying a lint rule

Comments

@nstarman
Copy link

nstarman commented Dec 30, 2023

In https://pep8.org/#module-level-dunder-names it says that __all__ "should be placed after the module docstring but before any import statements except from __future__ imports".
It would be great to have an isort-style rule that implements the PEP8 suggestion.
There are definitely cases when __all__ shouldn't be moved to above the imports, e.g. when it is dynamically defined from the imports, but if __all__ contains only strings then it should be safe to move.

import x, y, z

__all__ = ["x", "y", "z"]  # safe to move
from x import *
from . import x

__all__ = x.__all__  # not safe to move
@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Dec 31, 2023
@Skylion007
Copy link

For reference, isort already has this an option so we might just be able to add it to the isort rules: PyCQA/isort@44e3b5d

@charliermarsh
Copy link
Member

Does that open sort the members of __all__, or move the __all__ itself? I couldn't tell from the PR.

@alchzh
Copy link

alchzh commented Dec 31, 2023

For reference, isort already has this an option so we might just be able to add it to the isort rules: PyCQA/isort@44e3b5d

That is a different rule that is tracked by #1198. The title of this issue is a bit confusing. --srx sorts the strings inside of __all__, while this request is about moving the __all__ (and __version__, __author__, etc.) assignment statements to the top of the file, e.g.

Bad:

"""Dummy module"""

from __future__ import annotations

import os

__all__ = ["foo"]

Good:

"""Dummy module"""

from __future__ import annotations

__all__ = ["foo"]

import os

@nstarman , I think the title of this issue should be changed to something like "Add option to move module level dunders to top of file"

Does that open sort the members of __all__, or move the __all__ itself? I couldn't tell from the PR.

The former.

@alchzh
Copy link

alchzh commented Dec 31, 2023

It's worth noting that before 2016 the PEP8 mandated the opposite,

Put any relevant __all__ specification after the imports.

This was changed to the current text in
https://bugs.python.org/issue27187 (patch)
python/peps@0aa70ae

but the style checkers only relaxed the previous rule instead of implementing the new one PyCQA/pycodestyle#523 PyCQA/pycodestyle#615 , so there's been no pressure to change it in existing codebases. It seems like most Python code follows the older convention still, since people tend to go by whatever their style checker / formatter says instead of reading PEP8 themselves.

I don't know if there's any other PEP8 suggestion that so wildly differs from what's done in practice.

@nstarman nstarman changed the title Add option to sort __all__ according to PEP8 Add option to move module level dunders to top of file Dec 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

4 participants