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.

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

Jump to bottom

[Epic][CI] Migrate linter to Ruff #47991

Open
5 of 25 tasks
MortalHappiness opened this issue Oct 11, 2024 · 4 comments
Open
5 of 25 tasks

[Epic][CI] Migrate linter to Ruff #47991

MortalHappiness opened this issue Oct 11, 2024 · 4 comments
Assignees
@jjyao @CheyuWu @MortalHappiness @rynewang @aslonnie
Labels
bug Something that is supposed to be working; but isn't ci-test enhancement Request for new feature and/or capability

Comments

@MortalHappiness
Copy link
Member

MortalHappiness commented Oct 11, 2024 •
edited
Loading

What happened + What you expected to happen

While running the pre-commit hook of flake8, error occurs if Python version is 3.12. It's because the version of flake8 is too old.

We decided to fix this issue by migrating to ruff linter.

Versions / Dependencies

Python 3.12

Reproduction script

If python version is 3.9 -> no error

If python version is 3.12 -> has error

Upgrade the following packages to the newest version solves this issue:

  • flake8==7.1.1
  • flake8-comprehensions==3.15.0
  • flake8-quotes==3.4.0
  • flake8-bugbear==24.8.19

Issue Severity

Low: It annoys or frustrates me.

flake8 upgrade subtasks tracking:

Thanks for @CheyuWu 's comment here #48022 (comment)

pycodestyle

pyflakes

flake8-bugbear

https://github.com/PyCQA/flake8-bugbear

flake8-comprehensions

https://github.com/adamchainz/flake8-comprehensions

CI changes

Cleanup
@MortalHappiness MortalHappiness added bug Something that is supposed to be working; but isn't ci-test labels Oct 11, 2024
@CheyuWu
Copy link

CheyuWu commented Oct 11, 2024

@MortalHappiness I'd like to help with this!

@CheyuWu CheyuWu mentioned this issue Oct 13, 2024
Closed
8 tasks
@MortalHappiness MortalHappiness mentioned this issue Oct 15, 2024
Closed
@MortalHappiness MortalHappiness self-assigned this Oct 15, 2024
@MortalHappiness MortalHappiness changed the title [CI] flake8 version is too old to be compatible with Python 3.12 [Epic][CI] Upgrade flake8 to version 7.1.1 Oct 15, 2024
@MortalHappiness MortalHappiness added the enhancement Request for new feature and/or capability label Oct 15, 2024
@CheyuWu CheyuWu mentioned this issue Oct 16, 2024
Closed
8 tasks
This was referenced Oct 17, 2024
Open
Closed
Open
Open
Open
Open
Closed
Closed
Closed
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
This was referenced Oct 17, 2024
Open
Open
@MortalHappiness MortalHappiness mentioned this issue Oct 22, 2024
Closed
@LeoLiao123 LeoLiao123 mentioned this issue Oct 22, 2024
Merged
8 tasks
pcmoritz pushed a commit that referenced this issue Oct 23, 2024
@LeoLiao123
[CI] Fix flake8 rule E225: Missing whitespace around operator (#48188)
9c0064b 
<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->
See #47991 
When running the following `flake8` command to check for errors:
```
flake8 --select E225 --extend-exclude python/ray/core/generated,python/ray/serve/generated/,python/ray/cloudpickle/,python/ray/_private/runtime_env/_clonevirtualenv.py,doc/external/,python/ray/dashboard/client/node_modules
```
the following error occurs : 

![image](https://github.com/user-attachments/assets/e595a58e-677d-480f-9490-f52e62e4f0cf)



## Related issue number
Closes #48059 
<!-- For example: "Closes #1234" -->

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [x] 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 :(

Signed-off-by: LeoLiao123 <[email protected]>
@jjyao
Copy link
Collaborator

jjyao commented Oct 30, 2024

@MortalHappiness we should explore replacing flake8 with https://github.com/astral-sh/ruff if we want to do the upgrade.

@MortalHappiness
Copy link
Member Author

@jjyao Do you mean to replace flake8 completely with ruff?

@jjyao
Copy link
Collaborator

jjyao commented Oct 30, 2024 •
edited
Loading

Yes. Reasoning is that ruff is drop-in replacement for flake8 so upgrading flake8 and switching to ruff should require the similar amount of efforts.

@MortalHappiness MortalHappiness mentioned this issue Nov 2, 2024
Open
Jay-ju pushed a commit to Jay-ju/ray that referenced this issue Nov 5, 2024
@LeoLiao123 @Jay-ju
[CI] Fix flake8 rule E225: Missing whitespace around operator (ray-pr…
6611444 
…oject#48188)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->
See ray-project#47991 
When running the following `flake8` command to check for errors:
```
flake8 --select E225 --extend-exclude python/ray/core/generated,python/ray/serve/generated/,python/ray/cloudpickle/,python/ray/_private/runtime_env/_clonevirtualenv.py,doc/external/,python/ray/dashboard/client/node_modules
```
the following error occurs : 

![image](https://github.com/user-attachments/assets/e595a58e-677d-480f-9490-f52e62e4f0cf)



## Related issue number
Closes ray-project#48059 
<!-- For example: "Closes ray-project#1234" -->

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [x] 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 :(

Signed-off-by: LeoLiao123 <[email protected]>
JP-sDEV pushed a commit to JP-sDEV/ray that referenced this issue Nov 14, 2024
@LeoLiao123 @JP-sDEV
[CI] Fix flake8 rule E225: Missing whitespace around operator (ray-pr…
f11dadd 
…oject#48188)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->
See ray-project#47991 
When running the following `flake8` command to check for errors:
```
flake8 --select E225 --extend-exclude python/ray/core/generated,python/ray/serve/generated/,python/ray/cloudpickle/,python/ray/_private/runtime_env/_clonevirtualenv.py,doc/external/,python/ray/dashboard/client/node_modules
```
the following error occurs : 

![image](https://github.com/user-attachments/assets/e595a58e-677d-480f-9490-f52e62e4f0cf)



## Related issue number
Closes ray-project#48059 
<!-- For example: "Closes ray-project#1234" -->

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [x] 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 :(

Signed-off-by: LeoLiao123 <[email protected]>
mohitjain2504 pushed a commit to mohitjain2504/ray that referenced this issue Nov 15, 2024
@LeoLiao123 @mohitjain2504
[CI] Fix flake8 rule E225: Missing whitespace around operator (ray-pr…
34b3b26 
…oject#48188)

<!-- Thank you for your contribution! Please review
https://github.com/ray-project/ray/blob/master/CONTRIBUTING.rst before
opening a pull request. -->

<!-- Please add a reviewer to the assignee section when you create a PR.
If you don't have the access to it, we will shortly find a reviewer and
assign them to your PR. -->

## Why are these changes needed?

<!-- Please give a short summary of the change and the problem this
solves. -->
See ray-project#47991
When running the following `flake8` command to check for errors:
```
flake8 --select E225 --extend-exclude python/ray/core/generated,python/ray/serve/generated/,python/ray/cloudpickle/,python/ray/_private/runtime_env/_clonevirtualenv.py,doc/external/,python/ray/dashboard/client/node_modules
```
the following error occurs :

![image](https://github.com/user-attachments/assets/e595a58e-677d-480f-9490-f52e62e4f0cf)

## Related issue number
Closes ray-project#48059
<!-- For example: "Closes ray-project#1234" -->

## Checks

- [x] I've signed off every commit(by using the -s flag, i.e., `git
commit -s`) in this PR.
- [x] 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 :(

Signed-off-by: LeoLiao123 <[email protected]>
Signed-off-by: mohitjain2504 <[email protected]>
@MortalHappiness MortalHappiness changed the title [Epic][CI] Upgrade flake8 to version 7.1.1 [Epic][CI] Migrate linter to Ruff Dec 25, 2024
@MortalHappiness MortalHappiness mentioned this issue Dec 26, 2024
Open
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jjyao jjyao

@CheyuWu CheyuWu

@MortalHappiness MortalHappiness

@rynewang rynewang

@aslonnie aslonnie

Labels
bug Something that is supposed to be working; but isn't ci-test enhancement Request for new feature and/or capability
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Style: update flake8 to latest CheyuWu/ray
5 participants
@jjyao @CheyuWu @MortalHappiness @rynewang @aslonnie