Skip to content

vendor: three bespoke templates and AppType's dunders still use %-formatting, so the f-string disable cannot drop #798

Closed

Description

@JarryShaw

Describe the bug

#792 converted the six %-style raises in pcapkit/const/reg/apptype/apptype.py to f-strings, under the maintainer's convention from #783: "id like to keep f-string convention across the library. only use % substitution when inevitable." Three parts of that convention are still unmet, and nothing was tracking them until now.

1. Three other bespoke templates still raise with %. tests/const/test_const_enum_builtin_parity.py's BESPOKE_TEMPLATES names four modules that each carry their own copy of #647's _missing_ guard. #792 converted one; these three still hold the % form:

pcapkit.vendor.tcp.flags
pcapkit.vendor.ftp.command
pcapkit.vendor.http.method

Each is raise ValueError('%r is not a valid %s' % (value, cls.__name__)).

2. Three %-formatted dunders in the AppType template. __new__, __repr__, __str__. #792 left these deliberately and the reasoning is sound — __new__'s format sets the underlying StrEnum value for every one of ~12,391 real members, a far larger blast radius than an error path — so this wants care, not a sweep. Measure the emitted values before and after and prove them byte-identical, member by member.

3. The # pylint: disable=consider-using-f-string cannot drop until 1 and 2 are done, and also needs the extend_enum(..., '%d' % value, ...) calls in the span-handling tail converted. That disable being earned is the cheapest signal the work is unfinished.

Expected behavior

f-strings across all four bespoke templates and the AppType dunders, with the module-level disable dropped once nothing needs it.

Additional context

This issue exists because I cited the wrong one. #792's PR carries a comment reading "#796 tracks sweeping them" — but #796 is vendor: thirteen re.sub sites pass a regex flag as the positional count, an unrelated defect. I wrote that mis-citation into the worker's brief. The comment is being corrected to point here.

Also correcting #792 itself. Its "Expected behavior" said "f-strings throughout, and the disable dropped", which over-reached past its own title — that title scopes to "six pre-existing %-style raises". #792 is being narrowed to match, so it can close on the PR that does the six without its stated criteria going unmet. The remainder is this issue.

Two traps, both measured on #792's work:

  • The templates are generated, so edits go in pcapkit/vendor/..., where BASE is a lambda returning an f-string — braces in the emitted code must be doubled. :243's = {{}} is the precedent.
  • Do not regenerate. AppType.LINK fetches IANA's live CSV. Hand-edit and prove equivalence by rendering BASE(...) and diffing the changed region, expecting 0 diff lines.

And the lesson from #792's own 7-leg CI failure: grep tests/ for the literal text of every line touched, not only the constructs judged risky. A test pinned the guard's % text and nothing flagged it until CI went red — and note both tests in that class are skipUnless(find_spec('requests')), so they skip silently without the vendor extra.

Related: #792, #783, #647, #796.

Activity

  1. added
    refactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)
    constRegenerated IANA or vendor constant tables; members keep their numeric values
    on Sep 25, 2026
  2. JarryShaw commented on Sep 25, 2026

    @JarryShaw
    OwnerAuthor

    Labelling blocked so it does not read as unheld work.

    Checkable blocker: #797 merged. Part 2 of this issue — the three %-formatted dunders and the extend_enum(..., '%d' % value, ...) calls — lives in pcapkit/vendor/reg/apptype/apptype.py and its generated pcapkit/const/reg/apptype/apptype.py, and #797 (ecfb1cddd, 27✅/3⏭/0❌, awaiting the maintainer) is holding both right now. Two agents editing one generated file clobber each other with no error — the later write simply wins.

    gh pr view 797 -R JarryShaw/PyPCAPKit --json state,mergedAt
    

    Part 1 — the three other bespoke templates — does not overlap #797 and could in principle go first. Keeping the whole issue blocked anyway, because splitting it into two PRs against the same convention buys nothing and #797 is a merge away.

    Note #796 is blocked on the same PR, and its eight '%s %s' % sites are in the vendor crawlers rather than these templates — so #796 and this issue do not collide with each other, only with #797. Whichever goes first should re-derive its line numbers: #797's diff is small but every number in both bodies predates it.

  3. added
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 25, 2026
  4. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    and removed
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 25, 2026
  5. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 25, 2026
  6. added this to the 1.5 milestone on Oct 6, 2026
  7. moved this to Done in PyPCAPKiton Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    constRegenerated IANA or vendor constant tables; members keep their numeric valuesrefactorRestructuring for its own sake — neither a fix nor a new capability (refactor: prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions