-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
We may be able to ditch Black in favor of just Ruff #12255
Comments
WOW, I just ran Then I ran Black again and it took like 10 seconds or so. Conclusion: Ruff is just incredibly fast, but formats the occasional line differently than Black. It's more aggressive to keep line lengts below the maximum; e.g., we currently have with Black raise ValueError(
"Number of channels in the Info object (%s) and "
"the data array (%s) do not match. " % (len(pos["chs"]), data.shape[0])
+ info_help
) (3rd line exceeds line length limit!) and Ruff formats it as: raise ValueError(
"Number of channels in the Info object (%s) and "
"the data array (%s) do not match. "
% (len(pos["chs"]), data.shape[0])
+ info_help
) But there's also cases where it helps make full use of the line length limit, e.g., it changes this: raise ValueError(
"dtypes length mismatch. Provided: {0}, "
"Expected: {1}".format(len(dtypes), info.shape[1])
) to this: raise ValueError(
"dtypes length mismatch. Provided: {0}, " "Expected: {1}".format(
len(dtypes), info.shape[1]
)
) Didn't reduce the number of lines required, but makes line 2 longer. Should've merge the String, though?! |
no strong feeling here. Faster is better, for sure, and we should probably switch to Ruff eventually (also eliminates a dependency), but I'd be willing to wait a few more months to let the folks at Astral iron out a few more wrinkles in the Ruff formatter (such as the failure to merge the strings). Which, BTW, you should report upstream if it's not already been reported. Are the examples you give exhaustive in terms of the kinds of situations that are handled differently? |
I think Black does similar things if I'm not mistaken. The Ruff docs actually show this behavior in examples, so apparently it's not considered a bug. I still think it's kind of weird … See here: https://docs.astral.sh/ruff/formatter/black/#implicit-string-concatenations-in-attribute-accesses
I'm not sure, I only checked the changes the Ruff formatter proposes for a handful of the 39 files it wants to touch :) But yes, certainly no rush here! Just wanted to share this with you, esp. the speed was really impressive to me :) Thre's an exhaustive list of differences between Ruff and Black here: |
I have a good experience with ruff personally.
… Message ID: ***@***.***>
|
I like it, I'm 👍 for replacing Black with Ruff. The differences seem to be small, but they make sense (the list of differences contains good arguments why Ruff is doing it like that). In the case of string concatenation, users can always manually format it the way it looks best. |
What I'll do is I'll make a PR where we can see all the proposed changes and then evaluate how and whether to move forward, okay? :) |
See #12261 I think it looks good / reasonable |
I just checked our |
Ruff has started offering formatting functionality a couple of months back, in addition to just lining. They claim they tested it on large codebases like Django and it yields >99.9% identical results to Black. Differences are e.g. that it would complain about comments exceeding the max. line length, while Black doesn't.
https://astral.sh/blog/the-ruff-formatter
https://docs.astral.sh/ruff/formatter/
In any case, we could theoretically drop Black from our CI and pre-commit hooks and simply use Ruff for everything. This would speed up the pre-commit checks.
WDYT?
The text was updated successfully, but these errors were encountered: