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

A005 (builtin module shadowing) should ignore private modules #12949

Open
jab opened this issue Aug 17, 2024 · 5 comments · May be fixed by #14505
Open

A005 (builtin module shadowing) should ignore private modules #12949

jab opened this issue Aug 17, 2024 · 5 comments · May be fixed by #14505
Assignees
Labels
needs-decision Awaiting a decision from a maintainer preview Related to preview mode features rule Implementing or modifying a lint rule

Comments

@jab
Copy link
Contributor

jab commented Aug 17, 2024

I just updated from ruff 0.5.0 to 0.6.1, and had to add an ignore for A005 so that ruff check would continue to pass:

jab/bidict@57a0d0b

Without that ignore, it would fail as follows:

❯ pre-commit run --all-files --verbose --show-diff-on-failure ruff
ruff.....................................................................Failed
- hook id: ruff
- duration: 0.01s
- exit code: 1

bidict/_abc.py:1:1: A005 Module `_abc` is shadowing a Python builtin module
Found 1 error.

This check is failing because it treats even private builtin modules as not-to-be-shadowed, and Python provides a private _abc builtin:

❯ python -c 'import _abc'  # succeeds

I propose that private modules be excluded from this check.

@charliermarsh charliermarsh added rule Implementing or modifying a lint rule needs-decision Awaiting a decision from a maintainer preview Related to preview mode features labels Aug 18, 2024
@charliermarsh
Copy link
Member

@charliermarsh
Copy link
Member

I would say it's reasonable to flag these though. What do you think @AlexWaygood?

@jab
Copy link
Contributor Author

jab commented Aug 24, 2024

While we're waiting for others' feedback, I realized I didn't yet provide rationale for why private modules should be excluded from this check, so here is some:

Quoting from ruff's own https://docs.astral.sh/ruff/rules/import-private-name/ rule:

PEP 8 states that names starting with an underscore are private. Thus, they are not intended to be used outside of the module in which they are defined. Further, as private imports are not considered part of the public API, they are prone to unexpected changes, especially outside of semantic versioning.

I'm not sure how much ruff tries to harmonize conflicting rules that users have enabled, but this could be an opportunity to improve harmony.

Also, a lot of the builtin private modules are like _abc (which is where I hit this), in that:

  • they are "claiming" a private name that is general and useful in outside contexts (in the case of _abc, for example, any library that provides its own ABCs might want to put implementation details in its own private _abc.py)
  • (I put "claiming" in quotes because) they do not intend to be taking these names away from use in outside contexts; the fact that the private names are exported is an accident of implementation (plus Python culture: assume we're all "consenting adults" responsible users)

Another interesting observation is that, out of the dozens of private modules in the standard library, https://docs.python.org/3/py-modindex.html documents only two of them, _thread and _tkinter, further suggesting that the others are not intended to be directly imported, and outside contexts should also have those names available for internal use.

Click for list of builtin private modules in Python 3.9, in case it's helpful
_abc
_aix_support
_ast
_asyncio
_bisect
_blake2
_bootlocale
_bootsubprocess
_bz2
_codecs
_codecs_cn
_codecs_hk
_codecs_iso2022
_codecs_jp
_codecs_kr
_codecs_tw
_collections
_collections_abc
_compat_pickle
_compression
_contextvars
_crypt
_csv
_ctypes
_ctypes_test
_curses
_curses_panel
_datetime
_dbm
_decimal
_elementtree
_functools
_hashlib
_heapq
_imp
_io
_json
_locale
_lsprof
_lzma
_markupbase
_md5
_multibytecodec
_multiprocessing
_opcode
_operator
_osx_support
_peg_parser
_pickle
_posixshmem
_posixsubprocess
_py_abc
_pydecimal
_pyio
_queue
_random
_scproxy
_sha1
_sha256
_sha3
_sha512
_signal
_sitebuiltins
_socket
_sqlite3
_sre
_ssl
_stat
_statistics
_string
_strptime
_struct
_symtable
_sysconfigdata__darwin_darwin
_testbuffer
_testcapi
_testimportmultiple
_testinternalcapi
_testmultiphase
_thread
_threading_local
_tkinter
_tracemalloc
_uuid
_virtualenv
_warnings
_weakref
_weakrefset
_xxsubinterpreters
_xxtestfuzz
_zoneinfo

@jab
Copy link
Contributor Author

jab commented Aug 24, 2024

All that said, I'm fine with just keeping my...

[tool.ruff.lint.flake8-builtins]
builtins-allowed-modules = ["_abc"]

config if it's not worth making any changes here.

@MichaReiser
Copy link
Member

Excluding private modules does make sense to me, considering that there are that many. @AlexWaygood what's your take on this? I would then suggest to make the change so that the rule can be stabilized in the next minor.

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 preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants