--fix flag removes imports in __init__ files if there is a lack of __all__

[Czaki]

If __init__ file does not contain __all__ variable and --fix option is specified then ruff removes imports instead of create __all__ variable with include all variables imported from submodules.
Or report it as an error without modifications to files. And highlight lack of __all__ variable.

[charliermarsh]

Will take a look at this today!

[charliermarsh]

Some relevant discussion from Flake8: PyCQA/pyflakes#162, PyCQA/pyflakes#471

[charliermarsh]

A couple options:

1. Suppress these errors entirely in __init__.py (always ignore unused imports for those files).
2. Suppress these errors when __init__.py only contains imports (potentially confusing).
3. Keep these errors under F401, but don't autofix them.
4. Keep these errors under F401, but don't autofix them and add a custom message regarding __all__.
5. Move these errors to a new error code, and don't autofix them.

My preferences are either (1) or (4).

[Czaki]

I like 4.

[charliermarsh]

(Going out in v0.0.86, which is building now.)

[fsouza]

@charliermarsh any thoughts on instructing users to configure per-file-ignores instead? Or adding a dedicated flag/config option (similar to autoflake's --ignore-init-module-imports)?

I feel like that if an import error is reported is should be fixable?

[charliermarsh]

I’m open to making it a configuration setting!

[smackesey]

+1 for making it an option to autofix these. Our project (Dagster) has two types of __init__.py files (1) pure "export" files (i.e. they just import from submodules); (2) those that define a bunch of functions themselves (and so are more like standard modules). The rule as currently implemented prevents removing all of the unused imports from type (2), which can be frustrating.

FWIW, one can get around the conundrum of having intended-to-be-reexported symbols in __init__.py files appear to be unused (and flagged with F401) by using redundant aliases. That's what we do for top-level dagster:

from dagster._core.definitions.decorators.sensor_decorator import (
    asset_sensor as asset_sensor,
    sensor as sensor,
    multi_asset_sensor as multi_asset_sensor,
)
from dagster._core.definitions.decorators.source_asset_decorator import (
    observable_source_asset as observable_source_asset,
)
from dagster._core.definitions.dependency import (
    DependencyDefinition as DependencyDefinition,
    MultiDependencyDefinition as MultiDependencyDefinition,
    NodeInvocation as NodeInvocation,
)

We had to add this because pyright/pylance complains the symbols are private otherwise (since they are imported rather than defined in the module).

So since other popular tools require you to do this (or use __all__) to mark imported symbols as public, and this is also a developing Python standard, it seems reasonable to me to by default not treat __init__.py imports as special, and have users opt into the "don't fix" special-casing.

[charliermarsh]

Yeah, that makes sense. I'll fix this today.

[charliermarsh]

This is going out in v0.0.157 (building now), feedback welcome!

[mara004]

Can ignore-init-module-imports only be set in a configuration file, or is it also possible to pass this on the command line? (I'm new to ruff.)

[dhruvmanila]

Currently, it's not possible to pass arbitrary config keys as flag on the command-line but that's likely to change in the future (#4297 (comment)).

[ethanmsl]

Was this fix/improvement rolled back?

using ruff 0.0.292 and it has the same behavior as described originally : it complains about unused imports in init.py [F401].

And the --fix option merely deletes them.

[charliermarsh]

Do you have the relevant setting enabled? https://docs.astral.sh/ruff/settings/#ignore-init-module-imports

[cthorrez]

I do have the ignore-init-module-imports setting enabled in my pyproject.toml but it doesn't really "ignore" them. It doesn't fix them but it still fails the ruff run which is an issue if ruff is being used in a precommit hook.

with ignore-init-module-imports = false, it just removes the imports breaking my code

with ignore-init-module-imports = true I get this type of error blocking my commit

ruff.....................................................................Failed
- hook id: ruff
- exit code: 1

riix/utils/__init__.py:1:25: F401 `.data_utils.RatingDataset` imported but unused; consider adding to `__all__` or using a redundant alias
riix/utils/__init__.py:1:40: F401 `.data_utils.generate_matchup_data` imported but unused; consider adding to `__all__` or using a redundant alias
Found 2 errors.```

[joostsijm]

@cthorrez workaround for pre-commit hook picking up the error message is to ignore F401 error per file type like in the following pyproject.toml:

[tool.ruff.per-file-ignores]
"__init__.py" = ["F401"]

[vergenzt]

FYI all, I ended up writing an ast-grep rule to rewrite such imports in my codebase to use the redundant import alias. (We're just getting started with ruff.) Figured I'd share it here:

id: _
language: py
files:
- ./src/**/__init__.py

rule:
  all:

  - pattern: $NAME

  - inside:
      kind: import_from_statement
      stopBy: end

  - kind: dotted_name

  # has to NOT be the first dotted_name in the import statement (that's the source module)
  - follows:
      any:
      - has: { kind: dotted_name }
      - kind: dotted_name
      stopBy:
        kind: import_from_statement

transform:
  REDUNDANT_ALIAS:
    replace:
      source: $NAME
      replace: (.*)
      by: $1 as $1

fix: $REDUNDANT_ALIAS

[grahamannett]

@cthorrez workaround for pre-commit hook picking up the error message is to ignore F401 error per file type like in the following pyproject.toml:

[tool.ruff.per-file-ignores]
"__init__.py" = ["F401"]

this is very helpful and what I ended up using but seems like it is then required for any code base that uses ruff with F401 (or pyflakes rules) and __init__.py as a way to import classes/functions/etc from elsewhere.

Is this not the suggested project structure and there is a more modern/appropriate way to do that? I don't really think __all__ makes sense in this case

[zanieb]

Note we changed this behavior again recently in #10365, you can see more discussion there.

init.py as a way to import classes/functions/etc from elsewhere

What do you mean? I would say that __init__ is very much not intended for external imports unless you are intentionally exposing them as a part of your public interface in which case __all__ or a redundant alias import foo as foo are the appropriate ways to do so.

[linpan]

fix it ?