Skip to content

const(apptype): six pre-existing %-style raises remain where the tree convention is f-strings #792

Description

@JarryShaw

Describe the bug

The maintainer's convention, ruled on #783: "id like to keep f-string convention across the library. only use % substitution when inevitable."

Measured, so the gap is explicit:

scope raise …Error(f'… raise …Error('…' % …)
pcapkit/ whole tree 605 2
pcapkit/const/reg/apptype/apptype.py on ef859f776 0 6

So the tree convention really is f-strings and this generated module is the exception. #783 converted the one raise it introduced and deliberately left the six pre-existing ones, to keep a const diff over a 12,391-member file reviewable for the thing it was actually doing.

The six, on ef859f776: :92, :2308, :2368, :2378, :2478, :2484. Plus three %-formatted dunders (__new__, __repr__, __str__) that the same convention arguably covers.

Expected behavior

f-strings for the six raises named above, which is what this issue's title scopes it to.

Narrowed, 2026-09-25. This line originally read "f-strings throughout, and the module-level # pylint: disable=consider-using-f-string dropped once nothing needs it" — which over-reached past the title. "Throughout" also covers the three %-formatted dunders (__new__, __repr__, __str__) and the extend_enum(..., '%d' % value, ...) calls, and the disable cannot drop until those and three other bespoke templates are done. Left as written, this issue would close on a PR that meets its title but not its stated criteria. The remainder is #798.

Additional context

Two things whoever takes this needs to know, both measured:

  1. It is a generated file, so the edit goes in the template. pcapkit/vendor/reg/apptype/apptype.py:93 is BASE = lambda NAME, DOCS, FLAG, TABLE, MISS, MODL: f'''… — a lambda returning an f-string, not a str.format() template. Braces in the emitted code must therefore be doubled in the template; :243's = {{}} is the existing precedent.
  2. Do not regenerate. AppType.LINK fetches IANA's live CSV, so a crawl would rewrite thousands of unrelated member rows. Hand-edit both files and prove equivalence the way fix(const): refuse a multi-bit proto in AppType._dispatch rather than resolving it LSB-first (#759) #783 did — expand BASE and diff the changed region against the const file, expecting 0 diff lines.

Also verify each converted message is byte-identical to the % form it replaces. #783 checked that for its one raise (147 chars vs 147, string-equal); %r versus !r is the trap.

Related: #783, #759.

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: #783 merged. This issue edits pcapkit/const/reg/apptype/apptype.py and pcapkit/vendor/reg/apptype/apptype.py, and #783 (9aada6af5, review: good-to-go, awaiting the maintainer) rewrites the same _dispatch region of both. Starting now would either conflict on merge or silently clobber whichever landed second — the two agents editing one file failure mode, which produces no error at all.

    The line numbers in the body are pinned to ef859f776 and will shift once #783 lands, since its diff is +10/−10 above most of them. Re-derive them rather than trusting them:

    git grep -n "raise [A-Za-z_]*Error('" -- pcapkit/const/reg/apptype/apptype.py
    

    Unblocks the moment #783 is in.

  3. added
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 25, 2026
  4. JarryShaw commented on Sep 25, 2026

    @JarryShaw
    OwnerAuthor

    Unblocked — #783 merged at 16:21:43Z as f41e5f880, which was this issue's written blocker. Confirmed the fix is on main rather than inferring it from the merge:

    $ git show origin/main:pcapkit/const/reg/apptype/apptype.py | grep -n "if namespaces:"
    2416:        if namespaces:
    

    Every line number in this issue's body has shifted, exactly as the blocker warned. Re-derived on f41e5f880:

     93   raise ValueError('%r is not a valid %s' % (value, cls.__name__))
    2309  raise ValueError('%s holds no members; they belong to its per-transport '
    2375  raise ValueError('%r is not a valid port number for %s' % (key, cls.__name__))
    2420  raise ValueError('%r names no transport protocol registry of %s'
    2528  raise ValueError('%r is not a valid %s' % (value, cls.__name__))
    2534  raise ValueError('%r is not a valid %s' % (value, cls.__name__))
    

    So the body's :92, :2308, :2368, :2378, :2478, :2484 are all stale — use the six above, or better, re-derive again at whatever main is when the work starts. Still six, and :2420 is the trailing ValueError in _dispatch that #783 deliberately left alone.

    blocked off, wip on. Dispatching.

  5. 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
  6. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 25, 2026
  7. added this to the 1.5 milestone on 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