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

import-outside-top-level (PLC0415) is incompatible with flake8-tidy-imports.banned-module-level-imports #12803

Closed
Avasam opened this issue Aug 11, 2024 · 5 comments · Fixed by #15628
Labels
accepted Ready for implementation bug Something isn't working

Comments

@Avasam
Copy link
Contributor

Avasam commented Aug 11, 2024

Reframed issue:

import-outside-top-level (PLC0415) and banned-module-level-imports (TID253) are incompatible together.

ruff.toml

[lint]
preview = true
extend-select = [
	"TID253", # banned-module-level-imports, see banned-module-level-imports below
	"PLC0415", # import-outside-top-level
]

[lint.flake8-tidy-imports]
# Modules we want setuptools to be able to work without requiring
banned-module-level-imports = [
	"ctypes", # see #4461 for example
	"typing_extensions", # Useful for development, shouldn't be required at runtime
	# platform-specific imports
	"winreg",
]

Code:

import winreg # raises TID253

def win_only_method():
    import winreg # raises PLC0415

    print(winreg)
Original issue
  • List of keywords you searched for before creating this issue. Write them down here so that others can find this issue more easily and help provide feedback.
    PLC0415, top level module import, banned api
  • The current Ruff version (ruff --version): ruff 0.5.5

I came across a use-case where it would be preferable to check that certain imports are never imported directly at the module level. This is mainly for dependencies that should stay optional, or that are known to be be ostly and only useful in specific codepaths.

For example, setuptools could configure ctypes, typing_extensions, and more, to ensure that they're never unconditionally imported.

A check that the import is behind a conditional (if-else, try-except) or in a function, should be sufficient.

This is similar in idea to the https://docs.astral.sh/ruff/rules/#flake8-type-checking-tch (TCH001, TCH002, TCH003) rules.

This would conflict with https://docs.astral.sh/ruff/rules/import-outside-top-level/ (PLC0415). My original idea was to suggest an ignore list for PLC0415, but I think this could be pushed further with a rule that actually enforces it.
If this idea is rejected, I'd still like an ignore/allowlist for PLC0415 .

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Aug 11, 2024
@charliermarsh
Copy link
Member

I think this is pretty reasonable. I've implemented this as a custom lint rule (for tensorflow -- to ensure it's always imported lazily) in the past.

@charliermarsh charliermarsh added the accepted Ready for implementation label Aug 11, 2024
@charliermarsh
Copy link
Member

Any imports that are marked as required-top-level should also be ignored in PLC0415.

@Avasam
Copy link
Contributor Author

Avasam commented Aug 11, 2024

Any imports that are marked as required-top-level should also be ignored in PLC0415.

Do you mean "required-not-top-level" ?

Also I just found https://docs.astral.sh/ruff/rules/banned-module-level-imports/ , it seems to be doing exactly what I want. But it's not respected by PLC0415. So I'll reframe my request.

@Avasam Avasam changed the title Feature Request: Configure imports that shouldn't be a module-level import-outside-top-level (PLC0415) is incompatible with flake8-tidy-imports.banned-module-level-imports Aug 11, 2024
@charliermarsh
Copy link
Member

Yes sorry, that’s what I meant. Thanks for spotting that!

@charliermarsh charliermarsh added bug Something isn't working and removed rule Implementing or modifying a lint rule labels Aug 11, 2024
@charliermarsh
Copy link
Member

I’ll call it a bug that they’re aren’t compatible.

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

Successfully merging a pull request may close this issue.

2 participants