Reformat using ruff #312

jgroman · 2026-06-01T15:13:59Z

jgroman commented

2026-06-01 15:13:59 +00:00

This is just raw output of ruff format . preconfigured to line length 100, no other changes were made.

Tests are failing due to unrelated problem with missing sqlite libraries, fix was introduced in #311.

This is just raw output of `ruff format .` preconfigured to line length 100, no other changes were made. Tests are failing due to unrelated problem with missing sqlite libraries, fix was introduced in #311.

jgroman self-assigned this

2026-06-01 15:13:59 +00:00

jgroman added 1 commit

2026-06-01 15:14:00 +00:00

Reformat using ruff

Run tests and linters / test (pull_request) Failing after 2m42s

Details

Run tests and linters / lint (pull_request) Has been skipped

Details

ba94879489

jgroman added a new dependency

2026-06-01 15:14:30 +00:00

#306 Reformat codebase to satisfy CI linter check

jgroman requested review from kparal

2026-06-01 15:18:09 +00:00

adamwill approved these changes

2026-06-02 22:42:44 +00:00

adamwill left a comment

I scrolled through about half of this somewhat carefully then my eyes melted and I turbo-scrolled the rest. Couple of little notes but I think it's probably fine. It's 99% single quotes -> double quotes and rewrapping brace blocks. It might be interesting to try and separate those so we can see the less-common and possibly-more-interesting changes alone, but meh, not worth the effort most likely.

alembic/env.py Outdated

					
				@ -21,2 +22,3 @@

				target_metadata = db.metadata

				#target_metadata = None

				# target_metadata = None

probably this line can just be removed?

I proposed the whole alembic/ to be excluded.

But in general, this is a config file, so I assume the commented out value is the original default value, so there's slight value in it.

I proposed the whole `alembic/` to be excluded. But in general, this is a config file, so I assume the commented out value is the original default value, so there's slight value in it.

blockerbugs/_version.py Outdated

					
				@ -129,2 +132,2 @@

				                    "full-revisionid": None,

				                    "dirty": False, "error": None, "date": None}

				            return {

				                "version": dirname[len(parentdir_prefix) :],

not sure I love how it adds a space to this construct (it does it in several more places below), but if it's too much trouble to stop it doing that I guess we can live with it.

Ugh, that is ugly. This file should also be excluded, but it does the same e.g. in blockerbugs/__init__.py:152:

            environ["PATH_INFO"] = environ["PATH_INFO"][len(self.prefix) :]

I played with it a little, this is how ruff likes things:

a = [1, 2, 3]
b = a[0:1]
b = a[0:]
b = a[:1]
b = a[int(0) : int(1)]
b = a[int(0) :]
b = a[: int(1)]

I guess it does make some sense, but looks very unusual to me.

True to its name, there's no configuration option for it.

Ugh, that is ugly. This file should also be excluded, but it does the same e.g. in `blockerbugs/__init__.py:152`: ``` environ["PATH_INFO"] = environ["PATH_INFO"][len(self.prefix) :] ``` I played with it a little, this is how ruff likes things: ``` a = [1, 2, 3] b = a[0:1] b = a[0:] b = a[:1] b = a[int(0) : int(1)] b = a[int(0) :] b = a[: int(1)] ``` I guess it does make some sense, but looks very unusual to me. True to its name, there's no configuration option for it.

blockerbugs/controllers/api/validators.py Outdated

					
				@ -194,3 +190,4 @@

				            raise ValidationError("%s invalid input string" % (self.name))

				class UrlValidator(BaseValidator):

not really related to this PR, but wow, we should outsource this

😆 1

docs/source/conf.py Outdated

					
				@ -62,3 +63,3 @@

				# There are two options for replacing |today|: either, you set today to some

				# non-false value, then it is used:

				#today = ''

				# today = ''

maybe change these to double quotes for consistency? ditto below

Yet another config file where auto-formatting doesn't make sense, in my opinion.

adamwill force-pushed feature/306-2-ruff-reformat from ba94879489

Run tests and linters / test (pull_request) Failing after 2m42s

Details

Run tests and linters / lint (pull_request) Has been skipped

Details

to 8ae4acc4e8

Run tests and linters / test (pull_request) Successful in 2m57s

Details

Run tests and linters / lint (pull_request) Failing after 2s

Details

2026-06-02 22:46:41 +00:00

Compare

adamwill commented

2026-06-02 22:46:50 +00:00

I've rebased this since I merged the sqlite-devel test fix into develop.

kparal requested changes

2026-06-03 13:00:44 +00:00

Dismissed

kparal left a comment

Some files need to be excluded, as pointed out in #311. So I'm marking that now as a change request. Will continue with the review in the meantime.

kparal commented

2026-06-03 13:02:54 +00:00

@adamwill wrote in #312 (comment):

It's 99% single quotes -> double quotes and rewrapping brace blocks

So why don't we set quote-style = "preserve"? I know you hate inconsistency, but I don't see any downside in this, just upsides (not changing lines which don't need to be changed).

There are almost zero config options for black and ruff formatting, yet this is one of the very exceptions to the rule that ruff has. There has to be a good reason for it.

@adamwill wrote in https://forge.fedoraproject.org/quality/blockerbugs/pulls/312#issuecomment-794905: > It's 99% single quotes -> double quotes and rewrapping brace blocks So why don't we set `quote-style = "preserve"`? I know you _hate inconsistency_, but I don't see any downside in this, just upsides (not changing lines which don't need to be changed). There are almost zero config options for black and ruff formatting, yet this is one of the very exceptions to the rule that ruff has. There has to be a good reason for it.

kparal referenced this pull request

2026-06-03 13:48:47 +00:00

WIP: show meaningful changes with ruff format #314

kparal commented

2026-06-03 13:49:28 +00:00

It might be interesting to try and separate those so we can see the less-common and possibly-more-interesting changes alone

I did that in #314

> It might be interesting to try and separate those so we can see the less-common and possibly-more-interesting changes alone I did that in #314

kparal commented

2026-06-03 15:26:55 +00:00

I left some of my comments in #314. Overall it's fine (I guess...), except a couple of places where I really don't like the new vertical sprawl. Argparser definitions, testdata definitions, and tests themselves (sometimes).

The docs are not currently reformatted. Since we're going through this, I'd really like to redo them as well. Should it be a part of this PR (as yet another commit), or another PR, any preferences?

I left some of my comments in #314. Overall it's fine (I guess...), except a couple of places where I _really_ don't like the new vertical sprawl. Argparser definitions, testdata definitions, and tests themselves (sometimes). The docs are not currently reformatted. Since we're going through this, I'd really like to redo them as well. Should it be a part of this PR (as yet another commit), or another PR, any preferences?

jgroman commented

2026-06-04 07:48:21 +00:00

@kparal wrote in #312 (comment):

@adamwill wrote in #312 (comment):

It's 99% single quotes -> double quotes and rewrapping brace blocks

So why don't we set quote-style = "preserve"? I know you hate inconsistency, but I don't see any downside in this, just upsides (not changing lines which don't need to be changed).

There are almost zero config options for black and ruff formatting, yet this is one of the very exceptions to the rule that ruff has. There has to be a good reason for it.

I am fine with using single-quote string delimiter for this repo since the majority of code is using that.

@kparal wrote in https://forge.fedoraproject.org/quality/blockerbugs/pulls/312#issuecomment-795035: > @adamwill wrote in #312 (comment): > > > It's 99% single quotes -> double quotes and rewrapping brace blocks > > So why don't we set `quote-style = "preserve"`? I know you _hate inconsistency_, but I don't see any downside in this, just upsides (not changing lines which don't need to be changed). > > There are almost zero config options for black and ruff formatting, yet this is one of the very exceptions to the rule that ruff has. There has to be a good reason for it. I am fine with using single-quote string delimiter for this repo since the majority of code is using that.

jgroman commented

2026-06-04 08:23:13 +00:00

@kparal wrote in #312 (comment):

I left some of my comments in #314. Overall it's fine (I guess...), except a couple of places where I really don't like the new vertical sprawl. Argparser definitions, testdata definitions, and tests themselves (sometimes).

I am not sure how to proceed here. ruff formatter cannot be configured to produce such output. So I guess we drop the reformat and just fix linter problems? (I am fine with either outcome).

@kparal wrote in https://forge.fedoraproject.org/quality/blockerbugs/pulls/312#issuecomment-795123: > I left some of my comments in #314. Overall it's fine (I guess...), except a couple of places where I _really_ don't like the new vertical sprawl. Argparser definitions, testdata definitions, and tests themselves (sometimes). I am not sure how to proceed here. `ruff` formatter cannot be configured to produce such output. So I guess we drop the reformat and just fix linter problems? (I am fine with either outcome).

jgroman commented

2026-06-04 08:28:04 +00:00

@kparal wrote in #312 (comment):

The docs are not currently reformatted. Since we're going through this, I'd really like to redo them as well. Should it be a part of this PR (as yet another commit), or another PR, any preferences?

AFAIK ruff does not format docs so we should probably leave it to a completely different ticket. We could use tools like rstcheck and mdlint for that.

@kparal wrote in https://forge.fedoraproject.org/quality/blockerbugs/pulls/312#issuecomment-795123: > The docs are not currently reformatted. Since we're going through this, I'd really like to redo them as well. Should it be a part of this PR (as yet another commit), or another PR, any preferences? AFAIK `ruff` does not format docs so we should probably leave it to a completely different ticket. We could use tools like `rstcheck` and `mdlint` for that.

kparal commented

2026-06-04 10:36:11 +00:00

I am not sure how to proceed here. ruff formatter cannot be configured to produce such output. So I guess we drop the reformat and just fix linter problems? (I am fine with either outcome).

No, I wrote that mostly to receive your feedback, e.g. "yes it makes sense to suppress autoformat in these cases" or "no, you still can't appreciate the clarify it brings, let's reformat everything!" 😉️ Once we agree, we can suppress autoformat in certain parts of the file like this:
https://docs.astral.sh/ruff/formatter/#format-suppression

AFAIK ruff does not format docs

Sorry, by "docs" I meant docstrings+comments. Here I suggested trying docformatter for it. But I can also do it manually if needed. My goal is to use the full maximum line length (currently proposed for 100 chars) even for docstrings and comments, so that we eliminate some of that vertical sprawl that ruff is going to create.

> I am not sure how to proceed here. `ruff` formatter cannot be configured to produce such output. So I guess we drop the reformat and just fix linter problems? (I am fine with either outcome). No, I wrote that mostly to receive your feedback, e.g. "yes it makes sense to suppress autoformat in these cases" or "no, you still can't appreciate the clarify it brings, let's reformat everything!" 😉️ Once we agree, we can suppress autoformat in certain parts of the file like this: https://docs.astral.sh/ruff/formatter/#format-suppression > AFAIK `ruff` does not format docs Sorry, by "docs" I meant docstrings+comments. [Here](https://forge.fedoraproject.org/quality/blockerbugs/issues/306#issuecomment-760587) I suggested trying `docformatter` for it. But I can also do it manually if needed. My goal is to use the full maximum line length (currently proposed for 100 chars) even for docstrings and comments, so that we eliminate some of that vertical sprawl that ruff is going to create.

kparal commented

2026-06-04 11:05:46 +00:00

I suggested trying docformatter

Sigh, it doesn't seem to work correctly, at least in my attempts. So I guess I'll reflow it manually. But that will be a separate commit.

> I suggested trying `docformatter` Sigh, it doesn't seem to work correctly, at least in my attempts. So I guess I'll reflow it manually. But that will be a separate commit.

jgroman commented

2026-06-04 11:08:51 +00:00

@kparal wrote in #312 (comment):

No, I wrote that mostly to receive your feedback, e.g. "yes it makes sense to suppress autoformat in these cases" or "no, you still can't appreciate the clarify it brings, let's reformat everything!" 😉️ Once we agree, we can suppress autoformat in certain parts of the file like this: https://docs.astral.sh/ruff/formatter/#format-suppression

Oh, I see! Well, as I said, I do not have any problem even with no formatting at all so if such carve-outs are good enough for you, I say let's go for it! The question is how we will identify all such places in this large codebase?

AFAIK ruff does not format docs

Sorry, by "docs" I meant docstrings+comments. Here I suggested trying docformatter for it. But I can also do it manually if needed. My goal is to use the full maximum line length (currently proposed for 100 chars) even for docstrings and comments, so that we eliminate some of that vertical sprawl that ruff is going to create.

OK, then. I guess another PR would be useful for that. I am really interested whether it would make a big difference to reformat docstrings.

@kparal wrote in https://forge.fedoraproject.org/quality/blockerbugs/pulls/312#issuecomment-817656: > No, I wrote that mostly to receive your feedback, e.g. "yes it makes sense to suppress autoformat in these cases" or "no, you still can't appreciate the clarify it brings, let's reformat everything!" :wink:️ Once we agree, we can suppress autoformat in certain parts of the file like this: https://docs.astral.sh/ruff/formatter/#format-suppression Oh, I see! Well, as I said, I do not have any problem even with no formatting at all so if such carve-outs are good enough for you, I say let's go for it! The question is how we will identify all such places in this large codebase? > > > AFAIK `ruff` does not format docs > > Sorry, by "docs" I meant docstrings+comments. [Here](https://forge.fedoraproject.org/quality/blockerbugs/issues/306#issuecomment-760587) I suggested trying `docformatter` for it. But I can also do it manually if needed. My goal is to use the full maximum line length (currently proposed for 100 chars) even for docstrings and comments, so that we eliminate some of that vertical sprawl that ruff is going to create. OK, then. I guess another PR would be useful for that. I am really interested whether it would make a big difference to reformat docstrings.

kparal commented

2026-06-04 14:24:33 +00:00

The question is how we will identify all such places in this large codebase?

I reviewed it all already. The parts that I pointed out in #314 were the only ones that caught my attention. Once I learn Adam's opinion, I might try to suppress some parts of those files.

I guess another PR would be useful for that. I am really interested whether it would make a big difference to reformat docstrings.

I finally figured out that the bestest tool to understand and reformat docstrings and comments is... AI. It naturally understands it, so it reformats just the parts that need it. I'll prepare the PR.

I don't expect that many lines saved, to be honest. But just as some things provoke's Adam's perfectionism, some things provoke mine 😀️ If we're changing almost the whole codebase, we might as well do it completely...

But, I figured that perhaps we can use this step to actually sync the docstrings style. Some of our methods use Epytext, some Rest, some Google style. AI seems capable of rewriting it into a single style (I think Google is the best, now that we have type hints directly in method definitions). So I'll try to create a PR that also does that. So this will have more benefits than just basic reflowing.

> The question is how we will identify all such places in this large codebase? I reviewed it all already. The parts that I pointed out in #314 were the only ones that caught my attention. Once I learn Adam's opinion, I might try to suppress some parts of those files. > I guess another PR would be useful for that. I am really interested whether it would make a big difference to reformat docstrings. I finally figured out that the bestest tool to understand and reformat docstrings and comments is... AI. It naturally understands it, so it reformats just the parts that need it. I'll prepare the PR. I don't expect that many lines saved, to be honest. But just as some things provoke's Adam's perfectionism, some things provoke mine 😀️ If we're changing almost the whole codebase, we might as well do it completely... But, I figured that perhaps we can use this step to actually sync the [docstrings style](https://www.delftstack.com/howto/python/python-docstring-format/#docstring-patterns-in-python). Some of our methods use Epytext, some Rest, some Google style. AI seems capable of rewriting it into a single style (I think Google is the best, now that we have type hints directly in method definitions). So I'll try to create a PR that also does that. So this will have more benefits than just basic reflowing.

😆 1

kparal commented

2026-06-05 11:18:27 +00:00

@adamwill I might regret asking, but do you have any opinion on quote-style = "preserve" and my points from #314?

@adamwill I might regret asking, but do you have any opinion on [`quote-style = "preserve"`](https://forge.fedoraproject.org/quality/blockerbugs/pulls/312#issuecomment-795035) and my points from #314?

adamwill commented

2026-06-05 17:19:25 +00:00

I personally like having double quotes everywhere as it means I don't have to mentally adjust from project to project, but it's not a big deal. For 314, I replied there.

kparal referenced this pull request

2026-06-08 13:01:49 +00:00

Reformat codebase to satisfy CI linter check #306

kparal force-pushed feature/306-2-ruff-reformat from 8ae4acc4e8

Run tests and linters / test (pull_request) Successful in 2m57s

Details

Run tests and linters / lint (pull_request) Failing after 2s

Details

to 74eeb269ff

Run tests and linters / test (pull_request) Successful in 2m49s

Details

Run tests and linters / lint (pull_request) Failing after 2s

Details

2026-06-08 13:03:52 +00:00

Compare

kparal approved these changes

2026-06-08 13:08:36 +00:00

kparal left a comment

This is the final output. In the end, I didn't use any fmt: off or fmt:skip lines. Please give thumps up to approve. This can be merged once #311 is merged (and this will be rebased on top of it).

This is the final output. In the end, I didn't use any `fmt: off` or `fmt:skip` lines. Please give thumps up to approve. This can be merged once #311 is merged (and this will be rebased on top of it).

jgroman commented

2026-06-08 13:50:08 +00:00

LGTM

kparal referenced this pull request

2026-06-08 14:03:09 +00:00

WIP: show meaningful changes with ruff format #314

kparal force-pushed feature/306-2-ruff-reformat from 74eeb269ff

Run tests and linters / test (pull_request) Successful in 2m49s

Details

Run tests and linters / lint (pull_request) Failing after 2s

Details

to de4c2ba778