dify icon indicating copy to clipboard operation
dify copied to clipboard

enhancement: introduce Ruff for Python linter for reordering and removing unused imports with automated pre-commit and sytle check

Open bowenliang123 opened this issue 1 year ago • 4 comments

  • The Ruff tool is a general linting and formatting tools for Python code
  • Ruff is blazingly fast, as it finished lining or fixing all the hundreds of Python scripts of Dify project in less than a second, with the support of local cache and the core written in Rust
  • Ruff linter is designed as drop-in replacement for Flake8 (plus dozens of plugins), isort, pydocstyle, pyupgrade, autoflake, and more
  • To sort Python imports, the isort rules (I001, I002) in Ruff are applied (https://docs.astral.sh/ruff/rules/#isort-i)
    • although the line breaks in a style closer to black
  • Re-arranged Python imports in sections separated in source parties for better readability, in the order of "future", "standard-library", "third-party", "first-party", "local-folder". This greatly improves the readability and understandability.
  • To remove the unused imports, F401 rule of Flake8 in Ruff is applied (https://docs.astral.sh/ruff/rules/#pyflakes-f)
    • fixed 190 violations
    • the tests directory is skipped as the tests requires some unused-like imports for importing mocking methods

The key changes in this PR:

  1. apply ruff auto-fixes in one-key linting script dev/reformat
  2. automatically run linter scripts in pre-commit (via husky support) when committing changes, if API module changes
  3. check Python style in CI style workflow by running ruff linter and list all the violations if it fails

bowenliang123 avatar Feb 02 '24 22:02 bowenliang123

cc @takatost

bowenliang123 avatar Feb 03 '24 09:02 bowenliang123

I have a question. If the developer didn't initialize the pre-commit hook in the web directory, does that mean the Python reformat pre-commit hook won't work?

Yes. The workaround is that when the Python style CI job fails, it prints instructions to suggest the user to run dev/reformat which is clear and easy to apply.

As for the usage of Husky and other practices, they should be done in separated PR. As for this time, it's preferred to reuse the current Husky tool's support. The Git hooks require commit a pre-commit in .git/hooks/pre-commit which is ignored by default in .git, so I think we still need the necessary tool to automate the injection to the target script destination, with Husky or pre-commit.com tool or sth else.

bowenliang123 avatar Feb 04 '24 08:02 bowenliang123

Rebased and re-applied the Ruff fixes on to the latest commit of the master branch to resolve conflicts.

bowenliang123 avatar Feb 05 '24 07:02 bowenliang123

This is a new conflict in files.

Resolved.

bowenliang123 avatar Feb 05 '24 11:02 bowenliang123