astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
32.54k stars 1.08k forks source link

query: UP032 fix leaves bad formatting? #8683

Closed lucascolley closed 11 months ago

lucascolley commented 11 months ago

noted by @ilayn https://github.com/scipy/scipy/pull/19516#issuecomment-1810150463

version: ruff 0.1.5

UP032 fix seems to leave some undesired formatting. In the case of SciPy where we want to run the linter without a formatter, this is less than ideal. Is this the intended behaviour, or could it be improved? Examples:

- raise ValueError("Conflicting configuration dicts: {!r} {!r}"
-                  "".format(new_dict, d))
+ raise ValueError(f"Conflicting configuration dicts: {new_dict!r} {d!r}"
+                  "")

+ raise ValueError("invalid number of data points ({}) specified"
+                  .format(tmp.shape[axis]))
- raise ValueError(f"invalid number of data points ({tmp.shape[axis]}) specified"
-                  )
charliermarsh commented 11 months ago

Fix formatting is very much a best-effort thing, but we can definitely try to improve these. (I might argue that the first one had strange formatting before the fix too :))

lucascolley commented 11 months ago

I might argue that the first one had strange formatting before the fix too

Definitely 😆 I don't know how much code is formatted like this in the wider world but there are quite a few cases of this in scipy/scipy#19516

ilayn commented 11 months ago

first one had strange formatting before the fix too

Indeed and I am responsible for a few of those. But in our defense, the reasoning is the PEP8 80 chars line limit olympics.

We can live with the 2nd example though not really a big deal.

charliermarsh commented 11 months ago

It'd be nice to detect-and-strip trailing empty strings and trailing newlines.

charliermarsh commented 11 months ago

Hopefully better in the next release! At least, the two examples above are fixed by https://github.com/astral-sh/ruff/pull/8712.

ilayn commented 11 months ago

@charliermarsh thank you for the rapid action. Much appreciated.