Skip to content

Format Python code with ruff format instead of black #2824

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

Merged

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Apr 26, 2024

Failing tests are fixed in:

We want all tools run in pre-commit to be as fast as possible to enhance contributor productivity.

Let's consider formatting Python code with a highly compatible yet much faster formatter than Black.

https://docs.astral.sh/ruff/formatter/#philosophy

The formatter is designed as a drop-in replacement for Black, but with an excessive focus on performance and direct integration with Ruff. Given Black's popularity within the Python ecosystem, targeting Black compatibility ensures that formatter adoption is minimally disruptive for the vast majority of projects.

From: Known Deviations from Black

@cclauss cclauss requested a review from a team as a code owner April 26, 2024 11:20
@cclauss cclauss changed the title Format Python code with ruff instead of black Format Python code with ruff format instead of black Apr 26, 2024
@oddbookworm oddbookworm added Code quality/robustness Code quality and resilience to changes CI Issue with the Continuous Integration (CI), the actions/bots that test things labels Apr 26, 2024
Copy link
Member

@oddbookworm oddbookworm left a comment

Choose a reason for hiding this comment

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

Personally, I've been using ruff more lately, so I'm not against switching to it over black. But, I consider #2796 a blocker for this pull because of the failing lint check. I don't want every commit/pull to have a failing lint check while we wait on a fix pull to be merged.

I also would like to see opinions from the pygame-ce steering council.

@oddbookworm
Copy link
Member

@cclauss I haven't seen you on the pygame community discord server, where a lot of contribution discussions happen. If you'd like to join in those conversations, you can join the server at https://discord.gg/pygame

Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

OK, this looks good to me.

AFAICT, for our purposes ruff is just black (with a couple of super minor formatting differences) that will run faster because the formatting engine is re-written in a native code language (Rust in this case) rather than pure python.

No good reason not to have our formatter run faster (other than my general grumpy dislike of change.)

@MyreMylar MyreMylar closed this Apr 28, 2024
@MyreMylar MyreMylar reopened this Apr 28, 2024
@cclauss
Copy link
Contributor Author

cclauss commented Apr 28, 2024

a couple of super minor formatting differences

https://docs.astral.sh/ruff/formatter/#black-compatibility

@cclauss
Copy link
Contributor Author

cclauss commented Apr 28, 2024

Please re-run the failing test.

@cclauss cclauss requested a review from oddbookworm April 30, 2024 05:47
Copy link
Member

@oddbookworm oddbookworm left a comment

Choose a reason for hiding this comment

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

LGTM!

@cclauss
Copy link
Contributor Author

cclauss commented May 7, 2024

Do we need more approvals?

@damusss
Copy link
Member

damusss commented May 7, 2024

@cclauss Don't think so, I think someone just need to press some magical merge button

Copy link
Contributor Author

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

single-line-implicit-string-concatenation (ISC001)

…atenation


single-line-implicit-string-concatenation
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR 🎉

@ankith26 ankith26 added this to the 2.5.0 milestone May 8, 2024
@ankith26 ankith26 merged commit 810b1eb into pygame-community:main May 8, 2024
38 checks passed
@cclauss cclauss deleted the ruff-format-instead-of-black branch May 8, 2024 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Issue with the Continuous Integration (CI), the actions/bots that test things Code quality/robustness Code quality and resilience to changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants