astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.38k stars 1.08k forks source link

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

Open nstarman opened 10 months ago

nstarman commented 10 months ago

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
Skylion007 commented 10 months ago

For reference, isort already has this an option so we might just be able to add it to the isort rules: https://github.com/PyCQA/isort/commit/44e3b5d179da5033ce23f796b014e74f3a3259cd

charliermarsh commented 10 months ago

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

alchzh commented 10 months ago

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 commented 10 months ago

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) https://github.com/python/peps/commit/0aa70aebf102d352b8476f04509a369bf3db276c

but the style checkers only relaxed the previous rule instead of implementing the new one https://github.com/PyCQA/pycodestyle/pull/523 https://github.com/PyCQA/pycodestyle/issues/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.