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

Use Ruff for linting + fix linting errors #3774

Closed
wants to merge 1 commit into from

Conversation

JasonGrace2282
Copy link
Member

@JasonGrace2282 JasonGrace2282 commented May 20, 2024

See #3622
I'm also a big fan of Ruff, so I went ahead and replaced black and flake8 with it.

Status

  • Change pre-commit config
  • Fix linting errors

@JasonGrace2282 JasonGrace2282 added the maintenance refactoring, typos, removing clutter/dead code, and other code quality improvements label May 20, 2024
@JasonGrace2282 JasonGrace2282 marked this pull request as draft May 20, 2024 23:53
manim/utils/color/core.py Dismissed Show dismissed Hide dismissed
manim/mobject/mobject.py Fixed Show fixed Hide fixed
@JasonGrace2282 JasonGrace2282 changed the title Use Ruff for linting Use Ruff for linting + fix linting errors May 21, 2024
@JasonGrace2282 JasonGrace2282 marked this pull request as ready for review May 21, 2024 02:16
@JasonGrace2282 JasonGrace2282 linked an issue May 21, 2024 that may be closed by this pull request
@Viicos
Copy link
Member

Viicos commented May 21, 2024

Nice work! However, I don't see myself reviewing this in it's current form. There's way to much in a single PR.

Especially because ideally, the formatting changes will be added into the git-blame-ignore-revs file, and this is not possible in its current form as we use squash and merge.

I think this could be split into separate PRs, roughly:

  • Add ruff configuration in our pyproject.toml, but only enable formatting:
[tool.ruff]
line-length = "..."
extend-exclude = ["..."]  # Not sure if any extra excludes is required
  • In a separate PR, enable ruff format and remove black in the pre-commit config, apply formatting changes, merge.
  • In a separate PR, add the previous commit sha into the ignored blame revs.
  • Start adding lint:
    • Switch from isort to Ruff's isort (if too many changes, do the same as for formatting, and add the commit sha in the ignored blame revs).
    • Continue by adding pyflakes, pycodestyle, ruff, pyupgrade, etc. Preferably in separate PRs, so that we can ignore some commit SHAs if necessary

@JasonGrace2282
Copy link
Member Author

JasonGrace2282 commented May 21, 2024

I was thinking of that. My only real thought was that pre-commit ci would try to autofix and create a bigger difference than necessary (as originally, I just wanted one PR for pyproject/pre-commit changes and another PR for the diff).

Doesn't hurt to try, I'll see what happens

@Viicos
Copy link
Member

Viicos commented May 21, 2024

My only real thought was that pre-commit ci would try to autofix and create a bigger difference than necessary (as originally, I just wanted one PR for pyproject/pre-commit changes and another PR for the diff).

Yep, for this, you can add the necessary ruff configuration as I described, and then in a follow up PR make the change in the pre-commit file and let format everything (or let pre-commit do it)

@JasonGrace2282
Copy link
Member Author

JasonGrace2282 commented May 21, 2024

I'll just use this PR as a tracking PR then.
EDIT: Moved to the relevant issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance refactoring, typos, removing clutter/dead code, and other code quality improvements
Projects
Status: Rejected
Development

Successfully merging this pull request may close these issues.

Add linting back to CI
2 participants