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

[CI][Linter] Replace flake8 and isort with ruff #49435

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

MortalHappiness
Copy link
Member

@MortalHappiness MortalHappiness commented Dec 25, 2024

Why are these changes needed?

This PR replaces flake8 and isort with ruff.

To minimize changes, all rules that cause file modifications are ignored.

Although this PR involves many files, the changes to Python files are limited to modifications of flake8 comments.

The latest version of ruff does not support disabling checks for SyntaxError, so python/ray/serve/tests/test_config_files/syntax_error.py is explicitly excluded in pyproject.toml. See https://docs.astral.sh/ruff/rules/syntax-error/ for details.

Follow-up:

Related issue number

Closes #48508

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

@MortalHappiness MortalHappiness added the go add ONLY when ready to merge, run all tests label Dec 25, 2024
@MortalHappiness MortalHappiness force-pushed the feature/#48508-migrate-to-ruff branch 2 times, most recently from 18e5b64 to 18ead33 Compare December 25, 2024 10:47
@MortalHappiness MortalHappiness changed the title [CI][Linter] Migrate to ruff [CI][Linter] Replace flake8 and isort with ruff Dec 25, 2024
@MortalHappiness MortalHappiness force-pushed the feature/#48508-migrate-to-ruff branch from 3474ec9 to aab7d03 Compare December 25, 2024 11:27
@MortalHappiness MortalHappiness marked this pull request as ready for review December 25, 2024 15:20
MortalHappiness added a commit to MortalHappiness/ray that referenced this pull request Dec 25, 2024
@MortalHappiness MortalHappiness force-pushed the feature/#48508-migrate-to-ruff branch from aab7d03 to 2e84c6a Compare December 25, 2024 18:08
Copy link
Contributor

@dayshah dayshah left a comment

Choose a reason for hiding this comment

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

blazing fast linting ⚡️

hooks:
- id: ruff
args: [ --fix, --exit-non-zero-on-fix ]
# - id: ruff-format
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 will be uncommented in the follow-up PR to replace the black formatter.

# TODO(barakmich): This should be cleaned up. I've at least excised the copies
# of these arguments to this location, but the long-term answer is to actually
# make a flake8 config file
FLAKE8_PYX_IGNORES="--ignore=C408,E121,E123,E126,E211,E225,E226,E227,E24,E704,E999,W503,W504,W605"
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently, ruff does not support Cython.

In fact, flake8 also does not directly support Cython, which is why many rules need to be ignored here. If Cython support is still needed, this can be addressed in a follow-up PR by running ruff on Cython files, while similarly ignoring many rules.

Comment on lines 344 to +345
" image = self.pipe(prompt, num_inference_steps=50, guidance_scale=7.5).images[0]\n",
"\t\treturn image\n",
" return image\n",
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 fixes IndentationError.

@rynewang
Copy link
Contributor

cool! @aslonnie any comments?

@edoakes
Copy link
Contributor

edoakes commented Dec 26, 2024

Lonnie is OOO for a few weeks.

@can-anyscale any opposition to this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] Replace flake8 and isort with ruff
6 participants