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

feat: Replaced flake8 with ruff and removed setup.cfg by merging the configurations into pyproject.toml #27779

Merged
merged 1 commit into from
Dec 25, 2023

Conversation

Sai-Suraj-27
Copy link
Contributor

PR Description

Replaced flake8 with the following 2 major checks from ruff

  1. https://docs.astral.sh/ruff/rules/#pyflakes-f
  2. https://docs.astral.sh/ruff/rules/#pycodestyle-e-w

Removed setup.cfg by merging the required configurations into pyproject.toml

Note

  1. Fixed the E721 check : Do not compare types, use isinstance() rule wherever required.
  2. Changed the position of ruff in pre-commit hooks because sometimes, the fixes might modify files/lines that needs to be formatted by black again. So, placed ruff hook before the black hook.
  3. Added new noqa comments for 3 rules (E501 : line-to-long, E722 : Do not use bare except, and E741 : ambiguous-variable-name) and removed the old ones as ruff requires noqa comments to be added in a particular way.

Related Issue

Closes #27775

Checklist

  • Did you add a function?
  • Did you add the tests?
  • Did you run your tests and are your tests passing?
  • Did pre-commit not fail on any check?
  • Did you follow the steps we provided?

Socials

@Sai-Suraj-27
Copy link
Contributor Author

Requesting review from @vedpatwardhan and @KareemMAX.

@ivy-leaves ivy-leaves added Array API Conform to the Array API Standard, created by The Consortium for Python Data API Standards Ivy API Experimental Run CI for testing API experimental/New feature or request JAX Frontend Developing the JAX Frontend, checklist triggered by commenting add_frontend_checklist NumPy Frontend Developing the NumPy Frontend, checklist triggered by commenting add_frontend_checklist Ivy Functional API Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist PyTorch Frontend Developing the PyTorch Frontend, checklist triggered by commenting add_frontend_checklist labels Dec 21, 2023
Copy link
Contributor

@vedpatwardhan vedpatwardhan left a comment

Choose a reason for hiding this comment

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

Just a minor point, rest of the changes look good. Let's see what @KareemMAX thinks. Thanks @Sai-Suraj-27 😄

@@ -1,25 +0,0 @@
[flake8]
max-line-length = 88
ignore = E203, E402, E731, E704, W503, W504, W291, W293
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't fully know how this works, but shouldn't we be ignoring all of these in the pyproject.toml? In the pyproject.toml it seems to only be E203, E402 and E731

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, @vedpatwardhan
Yes, I didn't ignored those rules because of the following reasons,

  1. W291 : trailing whitespace warning
  2. W293 : blank line contains whitespace warning

These 2 should not be ignored + these 2 are taken care by the trailing-whitespace check which I introduced a few months back (#22974)
https://github.com/unifyai/ivy/blob/3c6d6afee9812fdca8ee73b5bb10b3c3660e4e49/.pre-commit-config.yaml#L6

  1. W503 : line break before binary operator
  2. W504 : line break after binary operator

Ruff didn't introduced these 2 checks in their checks because these 2 are really un-necessary rules that suggests us to have a line break before or after a binary operator.
image

  1. E704 : Multiple statements on one-line (def) (https://www.flake8rules.com/rules/E704.html)
    This check is still not implemented by ruff for some reason + I think this is a good check. So, we don't need to ignore it.

Copy link
Contributor

@KareemMAX KareemMAX left a comment

Choose a reason for hiding this comment

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

LGTM! I'll continue monitoring lint behavior to be sure

@KareemMAX KareemMAX merged commit 29ee162 into ivy-llc:main Dec 25, 2023
1149 of 1365 checks passed
zanieb pushed a commit to astral-sh/ruff that referenced this pull request Jan 31, 2024
…ection (#9735)

## Summary
I have recently made a contribution to a big python based repo
**replacing flake8 with ruff**
(ivy-llc/ivy#27779). So, as per this
[discussion](#9731). I am
making this PR to add the [ivy](https://github.com/unifyai/ivy)
repository (**with > 13k stars ⭐**) to the Ruff's readme

## Test Plan
No, need of any tests for the changes made.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Array API Conform to the Array API Standard, created by The Consortium for Python Data API Standards Ivy API Experimental Run CI for testing API experimental/New feature or request Ivy Functional API JAX Frontend Developing the JAX Frontend, checklist triggered by commenting add_frontend_checklist NumPy Frontend Developing the NumPy Frontend, checklist triggered by commenting add_frontend_checklist Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist PyTorch Frontend Developing the PyTorch Frontend, checklist triggered by commenting add_frontend_checklist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge setup.cfg into pyproject.toml
6 participants