[flake8-naming]: Respect import conventions (N817)
#12922
Conversation
MichaReiser
added
the
incompatibility
| | ^^^^^^^^^^^^^^^ N817 | ||
| | | ||
|
|
||
| N817.py:6:8: N817 CamelCase `ElementTree` imported as acronym `ET` |
There was a problem hiding this comment.
The configuration for this test run specifies that the convention for importing ElementTree is XET. That's why ET gets flagged.
There was a problem hiding this comment.
And the pre-existing test ensures that import ElementTree as ET is not flagged with the default settings, correct?
MichaReiser
force-pushed
the
fix-icn001-n817-incompatibility
branch
from
600c1f2
to
173cc67
Compare
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| N817 | 2 | 0 | 2 | 0 | 0 |
Linter (preview)
ℹ️ ecosystem check detected linter changes. (+0 -2 violations, +0 -0 fixes in 1 projects; 53 projects unchanged)
apache/airflow (+0 -2 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select ALL
- scripts/ci/testing/summarize_junit_failures.py:23:8: N817 CamelCase `ElementTree` imported as acronym `ET` - scripts/in_container/check_junitxml_result.py:21:8: N817 CamelCase `ElementTree` imported as acronym `ET`
Changes by rule (1 rules affected)
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| N817 | 2 | 0 | 2 | 0 | 0 |
| /// [`lint.flake8-boolean-trap.extend-allowed-calls`] option are allowed. | ||
| /// | ||
| /// |
There was a problem hiding this comment.
| /// [`lint.flake8-boolean-trap.extend-allowed-calls`] option are allowed. | |
| /// | |
| /// | |
| /// [`lint.flake8-import-conventions.banned-aliases`] option are allowed. | |
| /// |
Also, I notice you used square brackets but didn't add a link at the end. Was that deliberate -- do we have a mechanism that adds links automatically to settings?
There was a problem hiding this comment.
Yes, we have some fancy script that resolves settings (and validates them)
There was a problem hiding this comment.
Fancy script or no fancy script, you're still referring to the wrong setting here: the lint.flake8-boolean-trap.extend-allowed-calls setting has nothing to do with this rule
There was a problem hiding this comment.
I filed #12935 to address this
| | ^^^^^^^^^^^^^^^ N817 | ||
| | | ||
|
|
||
| N817.py:6:8: N817 CamelCase `ElementTree` imported as acronym `ET` |
There was a problem hiding this comment.
And the pre-existing test ensures that import ElementTree as ET is not flagged with the default settings, correct?
MichaReiser
changed the title
flake8-naming]: Respect import conventions (N8171)flake8-naming]: Respect import conventions (N817)
Summary
Fixes #12916 by respecting the configured import conventions in
N817.I decided to respect the conventions regardless on whether
ICN001is enabled or not because it makes the change easier to explain (fewer conditions when the behavior applies)and seems like a reasonable default.
Test Plan
Added tests
Breaking change
One could argue that this is a breaking change because
ETimports flagged before will now no-longer be flagged, requiring users to adopt their configuration if they want to keep flaggingET.I think it's fine but interested in more opinions.