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

Replace cpplint with clang-format #55150

Merged
merged 7 commits into from
Oct 22, 2023
Merged

Conversation

WillAyd
Copy link
Member

@WillAyd WillAyd commented Sep 15, 2023

cpplint isn't seemingly well maintained and focuses on C++, which we aren't using. I think clang-format is a much better tool for our formatting

@WillAyd WillAyd requested a review from noatamir as a code owner September 15, 2023 19:23
@WillAyd
Copy link
Member Author

WillAyd commented Sep 15, 2023

Since we are adding a hook to pre-commit I'm guessing pre-commit.ci will have to be updated somehow to account for it. Does @MarcoGorelli or @lithomas1 know how that works?

@lithomas1
Copy link
Member

Since we are adding a hook to pre-commit I'm guessing pre-commit.ci will have to be updated somehow to account for it. Does @MarcoGorelli or @lithomas1 know how that works?

I think Marco's out on vacation right now, so I'll do my best to help.

Can we try not using pocc's wrapper around clang-format? I think that one doesn't install clang-format for you, so it won't work on pre-commit CI.

You can try adding something manual like

hooks:
- id: pyright
# note: assumes python env is setup and activated
name: pyright
entry: pyright
language: node
pass_filenames: false
types: [python]
stages: [manual]
additional_dependencies: &pyright_dependencies
- [email protected]
- id: pyright
# note: assumes python env is setup and activated
name: pyright reportGeneralTypeIssues
entry: pyright --skipunannotated -p pyright_reportGeneralTypeIssues.json --level warning
language: node
pass_filenames: false
types: [python]
stages: [manual]
additional_dependencies: *pyright_dependencies
- id: mypy
# note: assumes python env is setup and activated
name: mypy
entry: mypy
language: system
pass_filenames: false
types: [python]
stages: [manual]
- id: mypy
# note: assumes python env is setup and activated
# note: requires pandas dev to be installed
name: mypy (stubtest)
entry: python
language: system
pass_filenames: false
types: [pyi]
args: [scripts/run_stubtest.py]
stages: [manual]
- id: inconsistent-namespace-usage
name: 'Check for inconsistent use of pandas namespace'
entry: python scripts/check_for_inconsistent_pandas_namespace.py
exclude: ^pandas/core/interchange/
language: python
types: [python]
- id: unwanted-patterns
name: Unwanted patterns
language: pygrep
entry: |
(?x)
# outdated annotation syntax
\#\ type:\ (?!ignore)
# foo._class__ instead of type(foo)
|\.__class__
# Numpy
|from\ numpy\ import\ random
|from\ numpy\.random\ import
# Incorrect code-block / IPython directives
|\.\.\ code-block\ ::
|\.\.\ ipython\ ::
# directive should not have a space before ::
|\.\.\ \w+\ ::
# Check for deprecated messages without sphinx directive
|(DEPRECATED|DEPRECATE|Deprecated)(:|,|\.)
# {foo!r} instead of {repr(foo)}
|!r}
# builtin filter function
|(?<!def)[\(\s]filter\(
types_or: [python, cython, rst]
exclude: ^doc/source/development/code_style\.rst # contains examples of patterns to avoid
- id: incorrect-backticks
name: Check for backticks incorrectly rendering because of missing spaces
language: pygrep
entry: '[a-zA-Z0-9]\`\`?[a-zA-Z0-9]'
types: [rst]
files: ^doc/source/
- id: seed-check-asv
name: Check for unnecessary random seeds in asv benchmarks
language: pygrep
entry: 'np\.random\.seed'
files: ^asv_bench/benchmarks
exclude: ^asv_bench/benchmarks/pandas_vb_common\.py
- id: unwanted-patterns-in-tests
name: Unwanted patterns in tests
language: pygrep
entry: |
(?x)
# pytest.xfail instead of pytest.mark.xfail
pytest\.xfail
# imports from pandas._testing instead of `import pandas._testing as tm`
|from\ pandas\._testing\ import
|from\ pandas\ import\ _testing\ as\ tm
# No direct imports from conftest
|conftest\ import
|import\ conftest
# pandas.testing instead of tm
|pd\.testing\.
# pd.api.types instead of from pandas.api.types import ...
|(pd|pandas)\.api\.types\.
# np.testing, np.array_equal
|(numpy|np)(\.testing|\.array_equal)
# unittest.mock (use pytest builtin monkeypatch fixture instead)
|(unittest(\.| import )mock|mock\.Mock\(\)|mock\.patch)
# pytest raises without context
|\s\ pytest.raises
# pytest.warns (use tm.assert_produces_warning instead)
|pytest\.warns
# os.remove
|os\.remove
# Unseeded numpy default_rng
|default_rng\(\)
files: ^pandas/tests/
types_or: [python, cython, rst]
- id: unwanted-patterns-in-cython
name: Unwanted patterns in Cython code
language: pygrep
entry: |
(?x)
# `<type>obj` as opposed to `<type> obj`
[a-zA-Z0-9*]>[ ]
types: [cython]
- id: pip-to-conda
name: Generate pip dependency from conda
language: python
entry: python scripts/generate_pip_deps_from_conda.py
files: ^(environment.yml|requirements-dev.txt)$
pass_filenames: false
additional_dependencies: [tomli, pyyaml]
- id: title-capitalization
name: Validate correct capitalization among titles in documentation
entry: python scripts/validate_rst_title_capitalization.py
language: python
types: [rst]
files: ^doc/source/(development|reference)/
- id: unwanted-patterns-bare-pytest-raises
name: Check for use of bare pytest raises
language: python
entry: python scripts/validate_unwanted_patterns.py --validation-type="bare_pytest_raises"
types: [python]
files: ^pandas/tests/
exclude: ^pandas/tests/extension/
- id: unwanted-patterns-private-function-across-module
name: Check for use of private functions across modules
language: python
entry: python scripts/validate_unwanted_patterns.py --validation-type="private_function_across_module"
types: [python]
exclude: ^(asv_bench|pandas/tests|doc)/
- id: unwanted-patterns-private-import-across-module
name: Check for import of private attributes across modules
language: python
entry: python scripts/validate_unwanted_patterns.py --validation-type="private_import_across_module"
types: [python]
exclude: |
(?x)
^(asv_bench|pandas/tests|doc)/
|scripts/validate_min_versions_in_sync\.py$
- id: unwanted-patterns-strings-with-misplaced-whitespace
name: Check for strings with misplaced spaces
language: python
entry: python scripts/validate_unwanted_patterns.py --validation-type="strings_with_wrong_placed_whitespace"
types_or: [python, cython]
- id: unwanted-patterns-nodefault-used-not-only-for-typing
name: Check that `pandas._libs.lib.NoDefault` is used only for typing
language: python
entry: python scripts/validate_unwanted_patterns.py --validation-type="nodefault_used_not_only_for_typing"
types: [python]
- id: use-pd_array-in-core
name: Import pandas.array as pd_array in core
language: python
entry: python scripts/use_pd_array_in_core.py
files: ^pandas/core/
exclude: ^pandas/core/api\.py$
types: [python]
- id: use-io-common-urlopen
name: Use pandas.io.common.urlopen instead of urllib.request.urlopen
language: python
entry: python scripts/use_io_common_urlopen.py
files: ^pandas/
exclude: ^pandas/tests/
types: [python]
- id: no-bool-in-core-generic
name: Use bool_t instead of bool in pandas/core/generic.py
entry: python scripts/no_bool_in_generic.py
language: python
files: ^pandas/core/generic\.py$
- id: no-return-exception
name: Use raise instead of return for exceptions
language: pygrep
entry: 'return [A-Za-z]+(Error|Exit|Interrupt|Exception|Iteration)'
files: ^pandas/
types: [python]
exclude: ^pandas/tests/
- id: pandas-errors-documented
name: Ensure pandas errors are documented in doc/source/reference/testing.rst
entry: python scripts/pandas_errors_documented.py
language: python
files: ^pandas/errors/__init__.py$
- id: pg8000-not-installed-CI
name: Check for pg8000 not installed on CI for test_pg8000_sqlalchemy_passthrough_error
language: pygrep
entry: 'pg8000'
files: ^ci/deps
types: [yaml]
- id: validate-min-versions-in-sync
name: Check minimum version of dependencies are aligned
entry: python -m scripts.validate_min_versions_in_sync
language: python
files: ^(ci/deps/actions-.*-minimum_versions\.yaml|pandas/compat/_optional\.py)$
additional_dependencies: [tomli, pyyaml]
pass_filenames: false
- id: validate-errors-locations
name: Validate errors locations
description: Validate errors are in appropriate locations.
entry: python scripts/validate_exception_location.py
language: python
files: ^pandas/
exclude: ^(pandas/_libs/|pandas/tests/|pandas/errors/__init__.py$|pandas/_version.py)
types: [python]
- id: future-annotations
name: import annotations from __future__
entry: 'from __future__ import annotations'
language: pygrep
args: [--negate]
files: ^pandas/
types: [python]
exclude: |
(?x)
/(__init__\.py)|(api\.py)|(_version\.py)|(testing\.py)|(conftest\.py)$
|/tests/
|/_testing/
- id: check-test-naming
name: check that test names start with 'test'
entry: python -m scripts.check_test_naming
types: [python]
files: ^pandas/tests
language: python
- id: sort-whatsnew-items
name: sort whatsnew entries alphabetically
entry: python -m scripts.sort_whatsnew_note
types: [rst]
language: python
files: ^doc/source/whatsnew/v
exclude: ^doc/source/whatsnew/v(0|1|2\.0\.0)

(see the pyright one.
We can try adding a new entry something like

id: clang-format
name: clang-format
entry: << clang-format command goes here >>
language: conda
additional_dependencies:
- clang-format

(I'm just typing this off of my head. Idea is to get pre-commit to pull clang-format off of conda. Don't expect it to work out of the box, though)

@WillAyd
Copy link
Member Author

WillAyd commented Sep 15, 2023

Ah OK thanks. I'll give that a shot - didn't realize pocc was missing something compared to the other repo to auto install

@mroeschke mroeschke added the Code Style Code style, linting, code_checks label Sep 18, 2023
@WillAyd
Copy link
Member Author

WillAyd commented Oct 3, 2023

Any concerns on this one?

Copy link
Member

@lithomas1 lithomas1 left a comment

Choose a reason for hiding this comment

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

Thanks Will,

This LGTM.

@MarcoGorelli any thoughts on this one?

@lithomas1 lithomas1 modified the milestones: 2.1.2, 2.2 Oct 4, 2023
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

sorry for the delay, looks really good, nice one!

@@ -166,6 +153,15 @@ repos:
types: [pyi]
args: [scripts/run_stubtest.py]
stages: [manual]
- id: clang-format
Copy link
Member

Choose a reason for hiding this comment

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

@mroeschke mroeschke merged commit b3a4b97 into pandas-dev:main Oct 22, 2023
33 checks passed
@mroeschke
Copy link
Member

Thanks @WillAyd

@cdce8p cdce8p mentioned this pull request Sep 27, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Style Code style, linting, code_checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants