networkx / nx-parallel

A networkx backend that uses joblib to run graph algorithms in parallel.
BSD 3-Clause "New" or "Revised" License
30 stars 21 forks source link

Pre-commit Linter is Inconsistent in Enforcing Code Formatting #81

Open dPys opened 1 week ago

dPys commented 1 week ago

Description It seems that the pre-commit hooks aren't enforcing linting consistently, as some code style changes were missed, even though the pre-commit checks passed.

Specifically, the following function in init:

def __init__(
    self,
    graph_object,
):

was not flagged for formatting, but it should have been a one-liner. The relevant pre-commit hooks (black, ruff, etc.) passed successfully despite this inconsistency.

Steps to Reproduce Make a formatting change like the above. Run the following pre-commit command:

pre-commit run --all-files --show-diff-on-failure --color always

Observe that the hooks pass, even though the code style is not consistent.

Expected Behavior The pre-commit hook should have failed, flagging the multi-line formatting in the init function and enforcing a consistent code style.

Actual Behavior The pre-commit hook passed without flagging the inconsistent formatting.

Environment Pre-commit configuration: blacken-docs: Passed prettier: Passed ruff: Passed ruff-format: Passed IDE: Possible IDE-related changes may also be contributing to this issue, but pre-commit should catch and enforce the linting regardless.

Proposed Fix We may want to: Review and refine the pre-commit configuration to ensure consistent enforcement of linting, especially with formatting rules. Align the current repo's pre-commit hooks with the ones used in the nx-parallel package for uniformity across all repositories. Update the versions of both ruff and black in the .pre-commit-config.yaml file, as they are currently outdated: Upgrade ruff to at least v0.5.1 (currently using v0.1.8). Upgrade black to at least v24.3.0 (to ensure up-to-date formatting standards). Confirm that the pre-commit checks properly catch multi-line issues like this in the future, and make necessary adjustments to hook configurations.

Other considerations We might consider also adding: Fixing end-of-file issues (end-of-file-fixer). Fixing trailing whitespaces (trailing-whitespace)

This would look as follows:

-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.3.0
    hooks:
    -   id: check-yaml
    -   id: end-of-file-fixer
    -   id: trailing-whitespace
MridulS commented 4 days ago

This may have fixed this https://github.com/networkx/nx-parallel/commit/c73066fbfd1d123a95bc842e44b8221ebc0f0cdc, we were installing a ruff inside the env and one in the pre-commit env :)

Schefflera-Arboricola commented 3 days ago

This may have fixed this c73066f, we were installing a ruff inside the env and one in the pre-commit env :)

But, the version of both the ruffs was same.