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

module-import-not-at-top-of-file ignore after sys.path.insert #5557

Closed
tgross35 opened this issue Jul 6, 2023 · 0 comments · Fixed by #9047
Closed

module-import-not-at-top-of-file ignore after sys.path.insert #5557

tgross35 opened this issue Jul 6, 2023 · 0 comments · Fixed by #9047
Labels
needs-decision Awaiting a decision from a maintainer rule Implementing or modifying a lint rule

Comments

@tgross35
Copy link
Contributor

tgross35 commented Jul 6, 2023

Sometimes you need to insert a path to be able to import something that's out of scope from your current file - in which case, the imports must come after the insert statement. For example, from Rust's repo:

from __future__ import absolute_import, division, print_function
import os
import unittest
# ...

bootstrap_dir = os.path.dirname(os.path.abspath(__file__))
sys.path.insert(0, bootstrap_dir)
import bootstrap # noqa: E402
import configure # noqa: E402

My suggestion is to ignore E402 module-import-not-at-top-of-file if it comes after a sys.path.insert statement.

There are legitimate uses of sys.path.insert, but it could be considered a hack or pitfall for new users - so I could understand potentially not wanting this behavior as the default. If this is the case, I would propose one of the following:

  • Create a configuration option such as ignore-after-sys-path-insert
  • Do this change as suggested, but create a separate lint that checks for sys.path.insert usage. The lint could theoretically be kind of smart and suggest possible alternatives for common misuses (adding __init__.py, importing directly, etc) but that kind of gets into evaluating the context
@charliermarsh charliermarsh added question Asking for support or clarification needs-decision Awaiting a decision from a maintainer rule Implementing or modifying a lint rule and removed question Asking for support or clarification labels Jul 9, 2023
charliermarsh added a commit that referenced this issue Dec 7, 2023
## Summary

It's common to interleave a `sys.path` modification between imports at
the top of a file. This is a frequent cause of `# noqa: E402` false
positives, as seen in the ecosystem checks. This PR modifies E402 to
omit such modifications when determining the "import boundary".

(We could consider linting against `sys.path` modifications, but that
should be a separate rule.)

Closes: #5557.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision Awaiting a decision from a maintainer rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants