Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Switch from black to ruff format#2550

Merged
pushfoo merged 5 commits into
pythonarcade:developmentpythonarcade/arcade:developmentfrom
cdeil:ruff-formatcdeil/arcade:ruff-formatCopy head branch name to clipboard
Feb 23, 2025
Merged

Switch from black to ruff format#2550
pushfoo merged 5 commits into
pythonarcade:developmentpythonarcade/arcade:developmentfrom
cdeil:ruff-formatcdeil/arcade:ruff-formatCopy head branch name to clipboard

Conversation

@cdeil

@cdeil cdeil commented Feb 2, 2025

Copy link
Copy Markdown
Contributor

This PR has three commits that are best reviewed one by one:

  • 5f03082 improves make.py a bit
  • 226e4e5 has some ruff format changes on arcade (not many changes, and very little ones removing empty lines or adding trailing commas mostly)
  • c1ac696 switches make.py, pyproject.toml and CONTRIBUTING.md from black to ruff format

Related: #2139, #2165, #2214

Looks like .pre-commit-config.yaml already only runs ruff format, no black.
So overall I think this is almost a drop-on replacement, just with some small gains in simplicity / consistency / speed.

Thoughts?

@einarf

einarf commented Feb 2, 2025

Copy link
Copy Markdown
Member

@cdeil

cdeil commented Feb 10, 2025

Copy link
Copy Markdown
Contributor Author

Yes? No? Maybe?

@einarf

einarf commented Feb 10, 2025

Copy link
Copy Markdown
Member

Probably. No resistance doing this change but properly mapping out the differences and double checking the config options is a good idea. Also need to make sure the tooling is up to date for the environments people are currently using.

Resolved a conflict with dev branch to keep this up to date.

@cdeil

cdeil commented Feb 10, 2025

Copy link
Copy Markdown
Contributor Author

There's infos on ruff format here: https://docs.astral.sh/ruff/formatter/

Basically it's 99.9% compatible with Black:

As such, 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.

The 0.1% deviations are explained here: https://docs.astral.sh/ruff/formatter/black/

I think you're already running ruff format and not black currently on precommit:

# Run the formatter.
- id: ruff-format

So this PR would align devdocs / CI in line and go all-in on ruff format vs the current solution of using two formatters and be faster. I think that's the main/only advantage, I would be surprised if anyone cares about the tiny formatting differences between the two.

Let me know if there's anything else I can do here.

@einarf

einarf commented Feb 10, 2025

Copy link
Copy Markdown
Member

Yup. We seems to have the majority of the maintains on board. I will adapt one of my local projects quickly first before I merging this just to get the full picture even if it's meant to be a drop-in replacement.

Will merge in the next few days.

The pre-commit I think was only a test that @eruvanos was using. It had some challenges depending on how your environment is set up. We should probably look into that as well soon.

@pushfoo

pushfoo commented Feb 10, 2025

Copy link
Copy Markdown
Member

I'm up for merging this.

  1. You fixed some weird fstring issues
  2. The root cause for them still isn't fixed in black Combine f-strings which would be placed on the same line psf/black#4389

@pushfoo

pushfoo commented Feb 17, 2025

Copy link
Copy Markdown
Member

#2569 just got hit with the exact issue I mentioned (f-string handling in psf/black#4389).

I will adapt one of my local projects quickly first before I merging this just to get the full picture even if it's meant to be a drop-in replacement.

@einarf any reasons not to merge this?

@einarf

einarf commented Feb 21, 2025

Copy link
Copy Markdown
Member

#2569 just got hit with the exact issue I mentioned (f-string handling in psf/black#4389).

I will adapt one of my local projects quickly first before I merging this just to get the full picture even if it's meant to be a drop-in replacement.

@einarf any reasons not to merge this?

Be free to merge. I have been swamped with work.

@pushfoo pushfoo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

TL;DR: Let's do it. 🚀

I have one minor personal style preference to gripe over, and that's handled by # fmt: off + # fmt: on if I really need it.

@pushfoo pushfoo merged commit 79defbf into pythonarcade:development Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.