Skip to content

STYLE ignore no-else-* checks, + assorted cleanups #49750

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Nov 23, 2022
Merged

STYLE ignore no-else-* checks, + assorted cleanups #49750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
04a1eeb
ignore no-else-*, comment some checks as intentionally turned off, sh…
Nov 17, 2022
93b5793
add redefined-outer-name check, but turn off in remaining files
Nov 17, 2022
6ec1012
keep redefined-outer-name to manual stage
Nov 19, 2022
e238fbb
Merge remote-tracking branch 'upstream/main' into no-else
Nov 19, 2022
312d64f
update exclude list
Nov 19, 2022
156ef11
reword
Nov 19, 2022
038a7e8
Merge remote-tracking branch 'upstream/main' into no-else
Nov 23, 2022
18f935e
Merge remote-tracking branch 'upstream/main' into no-else
Nov 23, 2022
255317a
fixup after merge
Nov 23, 2022
157ac22
fixup again
Nov 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/code-checks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ jobs:
- name: Run pre-commit
uses: pre-commit/[email protected]

typing_and_docstring_validation:
name: Docstring and typing validation
docstring_typing_pylint:
name: Docstring validation, typing, and pylint
runs-on: ubuntu-latest
defaults:
run:
Expand Down
21 changes: 20 additions & 1 deletion .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,25 @@ repos:
hooks:
- id: pylint
stages: [manual]
- repo: https://github.com/pycqa/pylint
rev: v2.15.5
hooks:
- id: pylint
alias: redefined-outer-name
name: Redefining name from outer scope
Copy link
Member

@jorisvandenbossche jorisvandenbossche Nov 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the idea of adding this one that only runs a specific check if we already have a pylint one for all checks above? (since both are labeled as "manual" stage)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one can't be applied to all files, it has lots of false positives in tests because of fixtures

but, like this, we can turn it on for the non-test files

unfortunately pylint doesn't have a per-file-excludes option like flake8 does

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, OK, thanks for the explanation!

files: ^pandas/
exclude: |
(?x)
^pandas/tests # keep excluded
|/_testing/ # keep excluded
|^pandas/util/_test_decorators\.py # keep excluded
|^pandas/_version\.py # keep excluded
|^pandas/conftest\.py # keep excluded
|^pandas/core/tools/datetimes\.py
|^pandas/io/formats/format\.py
|^pandas/core/generic\.py
args: [--disable=all, --enable=redefined-outer-name]
stages: [manual]
- repo: https://github.com/PyCQA/isort
rev: 5.10.1
hooks:
Expand Down Expand Up @@ -201,7 +220,7 @@ repos:
entry: python scripts/sync_flake8_versions.py
files: ^(\.pre-commit-config\.yaml|environment\.yml)$
pass_filenames: false
additional_dependencies: [pyyaml]
additional_dependencies: [pyyaml, toml]
Comment on lines -204 to +223
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unrelated to the pylint changes, but it's really minor so doing it while I'm here

The local hook just above has

    -   id: pip-to-conda
        name: Generate pip dependency from conda
        description: This hook checks if the conda environment.yml and requirements-dev.txt are equal
        language: python
        entry: python scripts/generate_pip_deps_from_conda.py
        files: ^(environment.yml|requirements-dev.txt)$
        pass_filenames: false
        additional_dependencies: [pyyaml, toml]

, so this one might as well take toml as a dep too and they'll share the same env

- id: title-capitalization
name: Validate correct capitalization among titles in documentation
entry: python scripts/validate_rst_title_capitalization.py
Expand Down
61 changes: 34 additions & 27 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,61 +58,70 @@ exclude = '''
[tool.pylint.messages_control]
max-line-length = 88
disable = [
"abstract-class-instantiated",
# intentionally turned off
"c-extension-no-member",
"comparison-with-itself",
"import-error",
"import-outside-toplevel",
"invalid-name",
"invalid-unary-operand-type",
"line-too-long",
"no-else-continue",
"no-else-raise",
"no-else-return",
"no-member",
"no-name-in-module",
"no-value-for-parameter",
"not-an-iterable",
"pointless-statement",
"redundant-keyword-arg",
"singleton-comparison",
"too-many-ancestors",
"too-many-arguments",
"too-many-boolean-expressions",
"too-many-branches",
"too-many-function-args",
"undefined-variable",
"too-many-instance-attributes",
"too-many-locals",
"too-many-nested-blocks",
"too-many-public-methods",
"too-many-return-statements",
"too-many-statements",
"unexpected-keyword-arg",
"unpacking-non-sequence",
"ungrouped-imports",
"unsubscriptable-object",
"unsupported-assignment-operation",
"unsupported-membership-test",
"unused-import",
"use-implicit-booleaness-not-comparison",
"use-implicit-booleaness-not-len",
"wrong-import-order",
"wrong-import-order",
"wrong-import-position",

# misc
"abstract-class-instantiated",
"redundant-keyword-arg",
"no-value-for-parameter",
"undefined-variable",
"unpacking-non-sequence",

# pylint type "C": convention, for programming standard violation
"import-outside-toplevel",
"invalid-name",
"line-too-long",
"missing-class-docstring",
"missing-function-docstring",
"missing-module-docstring",
"singleton-comparison",
"too-many-lines",
"ungrouped-imports",
"unidiomatic-typecheck",
"unnecessary-dunder-call",
"unnecessary-lambda-assignment",
"use-implicit-booleaness-not-comparison",
"use-implicit-booleaness-not-len",
"wrong-import-order",
"wrong-import-position",

# pylint type "R": refactor, for bad code smell
"comparison-with-itself",
"consider-using-ternary",
"consider-using-with",
"cyclic-import",
"duplicate-code",
"inconsistent-return-statements",
"no-else-return",
"redefined-argument-from-local",
"too-few-public-methods",
"too-many-ancestors",
"too-many-arguments",
"too-many-boolean-expressions",
"too-many-branches",
"too-many-instance-attributes",
"too-many-locals",
"too-many-nested-blocks",
"too-many-public-methods",
"too-many-return-statements",
"too-many-statements",
"unnecessary-list-index-lookup",

# pylint type "W": warning, for python specific problems
Expand All @@ -132,7 +141,6 @@ disable = [
"invalid-overridden-method",
"keyword-arg-before-vararg",
"overridden-final-method",
"pointless-statement",
"pointless-string-statement",
"possibly-unused-variable",
"protected-access",
Expand All @@ -148,7 +156,6 @@ disable = [
"unnecessary-lambda",
"unspecified-encoding",
"unused-argument",
"unused-import",
"unused-variable",
"using-constant-test",
"useless-parent-delegation"
Expand Down