Repository navigation
const: eight more files under const/vendor ftp+http still carry the f-string disable #818
Description
Describe the bug
Eight files under pcapkit/{const,vendor}/{ftp,http}/ still carry # pylint: disable=consider-using-f-string. #817 earns it for the command.py/method.py pair; these are the remainder.
Measured on 55e1b756e:
const/ftp/return_code.py const/http/error_code.py const/http/frame.py
const/http/setting.py const/http/status_code.py
vendor/ftp/return_code.py vendor/http/frame.py vendor/http/status_code.py (x2)
Expected behavior
f-strings throughout, and the disable dropped from every one — at which point the claim "nothing under pcapkit/{const,vendor}/{ftp,http}/ needs it" is finally true. #804's body asserted that prematurely; #817's author caught it, and this issue is the actual remainder.
Additional context
Three of the const/ files inherit their % from a shared template, not from their own generator: const/http/{error_code,frame,setting}.py come from pcapkit/vendor/default.py:55. So converting those three means editing the shared base, which emits far more than these eight files — check the blast radius across every generated const/ module before touching it, and expect that change to want its own PR.
Two traps, both hit on this family today:
- Count
%structurally, not textually. My own vendor: three %-formatted __repr__ methods keep the f-string disable alive in ftp.command and http.method #804 table saidvendor/ftp/command.pyhad 3%-format sites andvendor/http/method.py2; an AST walk overBinOp(Constant(str) % x)gives 1 each, because inside aLINE = lambda …: f'''template the__repr__lines are string content. And a%-grep must allow both quote styles —'[^']*%[sdr]reported 0 on files whose every site was double-quoted. - Check
tests/for assertions that pin the disable as RETAINED.test_const_enum_builtin_parity.py:658did exactly that for refactor(const): f-string the last three %-formatted __repr__ methods (#804) #817's pair (assertIn('consider-using-f-string', source)plusassertIn('%', source)), so it had to be flipped. Expect the same shape here, and flip rather than delete.
Use pylint with the real Makefile flag set plus useless-suppression to confirm each drop is earned — --disable=all --enable=useless-suppression manufactures false hits.
Activity
JarryShaw commentedon Sep 25, 2026
Checkable blocker: #817 merged.
#817 edits
tests/const/test_const_enum_builtin_parity.py(+269/−19), which is the file any work here must also touch — its:658assertion pins which modules retain the disable, so both changes rewrite the same assertions. #817 isreview: pendingwith a cross-review running, so the hold should be short.Note the three
const/http/{error_code,frame,setting}.pyfiles trace topcapkit/vendor/default.py:55, a shared template. If that turns out to want its own PR — and it probably does, given how many generated modules it emits — split this issue rather than growing it.JarryShaw commentedon Sep 25, 2026
Adding one item found by #817's cross-review:
pcapkit/const/http/method.py:2carries# pylint: disable=line-too-long, whichuseless-suppressionflags asI0021— a stale disable of a different rule, present on both stock and #817's head, so unrelated to the f-string work and not a regression.Worth sweeping in the same pass as the eight
consider-using-f-stringfiles, since the method is identical: runpylintwith the real Makefile flag set plus--enable=useless-suppressionoverpcapkit/const/andpcapkit/vendor/, and drop every disable it reports as useless — not just the f-string ones.make pylintcannot see any of this, becauseuseless-suppressionis not in its--enableset, which is worth deciding about separately: adding it would catch the next stale disable for free.Self-test whatever you use: a disable that is genuinely load-bearing must not be reported, or the sweep will strip working suppressions. #817's reviewer confirmed the two
consider-using-f-stringdisables it dropped were genuinely earned before and genuinely unneeded after, by checking both directions.JarryShaw commentedon Sep 25, 2026
Restating the blocker, because my own later comment displaced it from being the last one.
Checkable blocker: #817 merged.
#817 edits
tests/const/test_const_enum_builtin_parity.py(+269/−19), the same file any work here must touch — itsdroppedtuple is what pins which modules must not carry the disable, and this issue adds eight more to it. #817 isreview: good-to-gowith CI green, so the hold should be short.Scope as it now stands: the eight
consider-using-f-stringfiles, plus the stale# pylint: disable=line-too-longatconst/http/method.py:2thatuseless-suppressionflags asI0021on both sides of #817. Three of the eight —const/http/{error_code,frame,setting}.py— trace to the sharedpcapkit/vendor/default.py:55template, so split those out rather than growing this issue.Noting the mechanism for future reference: a blocker line only counts while it is in the last comment, so any comment added afterwards silently breaks the check. That is the same shape as the
cr:trailer problem — appending a paragraph demotes it.JarryShaw commentedon Sep 26, 2026
Unblocked — #817 merged at 02:43:34Z as
75c340413, which was the checkable blocker on this issue.So the eight remaining files under
pcapkit/const/andpcapkit/vendor/that still carry a%-formatted__repr__are now free to convert — #817 took theftp.commandandhttp.methodpair, and this is the remainder it deliberately left out of scope.Dispatching a worker. One thing for whoever picks it up:
mainmoved twice more after #817 (21e9588af, then75c340413), and two open PRs are live in nearby territory — #827 inpcapkit/corekit/fields/field.pyand #828's fix inpcapkit/corekit/fields/numbers.py— so branch from currentmainand stay insideconst/andvendor/.JarryShaw commentedon Sep 26, 2026
Correcting this issue's own scope — "eight files" was wrong in both directions. Found by #830's author, verified by me.
Over-counted.
pcapkit/const/http/{error_code,frame,setting}.pyhave no__repr__at all —grep -c 'def __repr__'is 0 for each, against 1 forconst/ftp/return_code.pyas a control. Their%lives in_missing_'sValueError/extend_enumcalls, inherited frompcapkit/vendor/default.py's shared template, which is a different change with a much wider blast radius.Under-named.
{const,vendor}/pcapng/option_type.pydoes carry a%-formatted__repr__and this issue never mentioned it, because I scoped the issue to{const,vendor}/{ftp,http}/from #817's remainder rather than from a search.So the real set is three pairs, six files:
{const,vendor}/ftp/return_code.py,{const,vendor}/http/status_code.py,{const,vendor}/pcapng/option_type.py— which is what #830 converts.One thing #830 deliberately does not do, and is right not to. #817 removed the module-level
consider-using-f-stringdisable from its pair, because after the conversion nothing in those files needed it. Here the disable must stay — measured on #830's head, each file still has other%-format sites:Those are
__str__and_missing_, which legitimately keep%. Removing the disable would redmake pylint, so the asymmetry with #817 is correct rather than an oversight.Still open after #830, and not folded in: the shared
vendor/default.pyheader disable, and theuseless-suppressionsweep for staleline-too-longonconst/http/method.py:2. Those want their own issue.JarryShaw commentedon Sep 26, 2026
Correcting my own comment above — the pragma reasoning was wrong for three of the six files.
I wrote that removing
consider-using-f-stringwould redmake pylinton all six, citing per-file%-site counts of5 / 10 / 5 / 5 / 4 / 5. Both halves of that are wrong, found by #830's cross-review and re-derived by me:pylint --enable=consider-using-f-stringwith the pragma stripped, not from grep) are5 / 0 / 10 / 0 / 6 / 0forconst/ftp,vendor/ftp,const/http,vendor/http,const/pcapng,vendor/pcapng. My values were transposed across the pairs.vendor/files have no module-level pragma at all. Theirs sits inside theLINE = lambdatemplate — pragma at:32/:28/:31, template starting at:29/:25/:28— so it is template text destined for the generatedconst/file. Measured:pragmas BEFORE the template: 0for all three, against1forconst/ftp/return_code.py, which has no template; and a strippedvendor/ftp/return_code.pymakes pylint fire 0 while a strippedconst/ftp/return_code.pyfires 6.The conclusion is unchanged — no dead pragma on any of the six, so #830 correctly changes none of them — but "removing it would red pylint" is only true of the three
const/files. On the vendor half there is nothing to remove.Worth recording why I got it wrong, because it is the third time today: I counted with a grep where the answer is structural. My column-0-anchored grep reported a module pragma in all six, and it was fooled because the template's own lines also begin at column 0 inside the triple-quoted string. The same class of error produced the bad
%-site counts here, the wrong disable-removal count on #817's diff, and the eight-vs-three call-site list on #828.test: name tests, constants and labels after the behaviour (#2325)