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

chore: Update Python development tools and unify Black into Ruff #1771

Merged
merged 14 commits into from
Dec 8, 2023

Conversation

achimnol
Copy link
Member

@achimnol achimnol commented Dec 8, 2023

  • setup: Update python tool dependencies
    • black: 23.9.1 -> completely replaced with ruff
      • The tool resolve still remains to prevent crashing of black initialization in our CI that runs pants tailor, which implicitly installs the black to scan BUILD files.
    • mypy: 1.6.1 -> 1.7.1
    • ruff: 0.1.1 -> 0.1.7
    • pytest: 7.4.2 -> 7.4.3

Note

You need to re-run:
pants export --resolve=black --resolve=mypy --resolve=ruff --resolve=black
to use the latest tool versions with your IDE or editor with LSPs.

Note

Note that black is deprecated but still remains as a dependency resolve set (tools/black-requirements.txt and tools/black.lock) to prevent Pantsbuild from failing on setting up pants tailor command when linting BUILD files.

Important

Ruff now provides an intrinsic formatter that works almost in the same way Black does.
Now Ruff replaces Black completely, so please DISABLE the black formatter but ENABLE the ruff formatter in your IDE and editors.

  • style: Apply the updated black and ruff rules
    • Now we can use more concise nested parenthesis/braces/brackets inside another one
      when there are no sibling items.
    • Actually this was my preferred style before adopting Black!
  • fix: newly found typecheck errors
    • mypy found some potential bugs with its improved type checking!

Tip

This PR changes a wide range of source files as we replace the formatter implementation.

When you encounter conflicts during merging the updated main branch to your topic branch:

  • Run git checkout --ours to keep your work.
  • Reformat everything by pants fix :: && pants fmt ::.

If you still see previously unobserved type errors, refer the commit 805128c to fix them.

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version

@achimnol achimnol added this to the 23.09 milestone Dec 8, 2023
@achimnol achimnol added area:infrastructure Infrastructure-related issues type:maintenance Maintenance issue. skip:changelog Make the action workflow to skip towncrier check impact:invisible This change is invisible to users (internal changes). labels Dec 8, 2023
@achimnol achimnol self-assigned this Dec 8, 2023
@github-actions github-actions bot added comp:client Related to Client component comp:manager Related to Manager component comp:storage-proxy Related to Storage proxy component comp:webserver Related to Web Server component require:db-migration Automatically set when alembic migrations are added or updated size:XL 500~ LoC labels Dec 8, 2023
@achimnol achimnol mentioned this pull request Dec 8, 2023
9 tasks
@achimnol achimnol removed skip:changelog Make the action workflow to skip towncrier check require:db-migration Automatically set when alembic migrations are added or updated labels Dec 8, 2023
@achimnol
Copy link
Member Author

achimnol commented Dec 8, 2023

NOTE: require:db-migration is a false-positive as black reformats some alembic migration scripts.

@achimnol achimnol added the type:bug Reports about that are not working label Dec 8, 2023
@achimnol
Copy link
Member Author

achimnol commented Dec 8, 2023

Currently I'd like to hold merging of this PR due to:

@achimnol
Copy link
Member Author

achimnol commented Dec 8, 2023

Ah, we can use https://docs.astral.sh/ruff/settings/#format-preview !
Ruff now provides an intrinsic formatter that works almost in the same way that Black does, even including some subset of preview styles.

But still there are some deviations, and the outcome is different for some complex (in terms of style) files like src/ai/backend/common/config.py.

- Now ruff provides a full-fledged black compatible formatter on its
  own!
- You may even disable the Black formatter completely in your IDE and
  editor to speed up your coding experience.
@github-actions github-actions bot added the require:db-migration Automatically set when alembic migrations are added or updated label Dec 8, 2023
@achimnol
Copy link
Member Author

achimnol commented Dec 8, 2023

For now, let's stay on the 2023 stable style whereas both ruff and black almost identically works.

@achimnol
Copy link
Member Author

achimnol commented Dec 8, 2023

Decided to commence a complete transition from black to ruff in favor of speed. (We are mono-repo with 670+ files to lint.)

Backported and vendored the Ruff plugin with proper "formatter" support from Pantsbuild 2.20.0dev0. Now all our pants lint, pants fix, pants fmt works with Ruff.

@achimnol achimnol added this pull request to the merge queue Dec 8, 2023
Merged via the queue into main with commit f95020d Dec 8, 2023
25 checks passed
@achimnol achimnol deleted the chore/update-pytools branch December 8, 2023 15:09
achimnol added a commit that referenced this pull request Dec 8, 2023
Backported-from: main
Backported-to: 23.09
@achimnol achimnol changed the title chore: Update Python development tools chore: Update Python development tools and unify Black into Ruff Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:infrastructure Infrastructure-related issues comp:client Related to Client component comp:manager Related to Manager component comp:storage-proxy Related to Storage proxy component comp:webserver Related to Web Server component impact:invisible This change is invisible to users (internal changes). require:db-migration Automatically set when alembic migrations are added or updated size:XL 500~ LoC type:bug Reports about that are not working type:maintenance Maintenance issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant