Skip to content

const: make EnumRegistry.get's NO_DEFAULT a sentinel object, not -1 #857

Closed

Description

@JarryShaw

Is your feature request related to a problem? Please describe.

EnumRegistry.get's no default marker is the magic value -1, named as NO_DEFAULT in
pcapkit/corekit/enums.py:64 but still just an int. Raised by @JarryShaw on #855: "I'm wondering if we should
change the NO_DEFAULT to a sentinel object."

There is no correctness bug today, measured on #855's head cb7eb0d94:

NO_DEFAULT consumers            : 1 file, 5 lines  (pcapkit/corekit/enums.py:64,103,123,140,146)
generated registries still on -1: 108 files, each its own `default: 'int' = -1`
min value across all const      : 0    -> -1 is unreachable as a legitimate value

(last line from grep -rhoE '^ [A-Z_0-9]+ = -?[0-9]+$' pcapkit/const | grep -oE '\-?[0-9]+$' | sort -n | head)

The wart it does have: the test is default == NO_DEFAULT, and -1.0 == -1 is True, so get(key, -1.0)
silently means no default rather than defaulting to -1.0. Degenerate — no caller passes a float — but it is
the class of thing a sentinel makes impossible.

Describe the solution you'd like

NO_DEFAULT = object(), with the two comparisons at enums.py:140 and :146 becoming is rather than ==. An
object() cannot compare equal to anything a caller might pass, and no call site needs a new import, since
default keeps its default value. A subclass or an enum member would both be worse — each can still compare equal
to something.

Describe alternatives you've considered

Doing it inside #855, which is where it was raised. Rejected: #855 already leaves pcapkit.const with two
contradictory register contracts
— the base guards an already-registered value, the 105 generated registries
still silently alias — and changing only the base's get would add a second base-vs-generated divergence.

Additional context

This belongs to the tier of #775 that rewrites pcapkit/vendor/default.py and regenerates, because that is the
only place the base and the 108 generated registries change atomically. Fold it in beside the register() guard
residue, which has the identical shape.

Deferred by @JarryShaw on #855: "Okay let's save the sentinel change to an issue (existing or new)." Also
recorded as a scoped item on #775.

Activity

  1. added
    enhancementIssues requesting a new capability (set by the feature request template)
    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
    blockedDeferred pending another issue or decision; see the last comment for what unblocks it
    on Sep 27, 2026
  2. JarryShaw commented on Sep 27, 2026

    @JarryShaw
    OwnerAuthor

    Checkable blocker. This waits on the tier of #775 that rewrites pcapkit/vendor/default.py and regenerates the
    const registries, because the base and the 108 generated default: 'int' = -1 signatures have to change together —
    doing the base alone adds a base-vs-generated divergence rather than removing one.

    gh issue view 775 -R JarryShaw/PyPCAPKit --json state -q .state        # must be CLOSED, or
    git grep -c "default: 'int' = -1" -- pcapkit/vendor/default.py         # must be 0

    Either condition clearing means this can start. Right now #775 is OPEN and vendor/default.py still carries
    the -1 signature, so neither has.

    Cross-linked from #855 (where it was raised and deferred) and #775 (scoped item).

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

    @JarryShaw
    OwnerAuthor

    Unblocked — #858 merged at 18:38:40Z and it was the gate. Both conditions on the blocker above now read clear:

    git grep -c "default: 'int' = -1" -- pcapkit/vendor/default.py   ->  0
    pcapkit/vendor/default.py:68   from pcapkit.corekit.enum import EnumRegistry
    pcapkit/vendor/default.py:73   class {NAME}(EnumRegistry, IntEnum):
       ... and no `def get`, `def register` or `def _unregistered_member` emitted at all
    

    So the template no longer carries its own get, and 111 of 121 const modules now inherit NO_DEFAULT
    behaviour from the single base — up from 6 before #858. The 10 that do not are the bespoke __new__ set
    (ftp/command, ftp/return_code, http/method, http/status_code, pcapng/option_type) plus reg/apptype/*,
    none of which share the template.

    That is what makes this actionable now: changing NO_DEFAULT in pcapkit/corekit/enum.py is a one-file
    change that reaches all 111, instead of the base-versus-generated divergence it would have created before #858.

    Also note the module moved: it is pcapkit/corekit/enum.py (no -s) since #858, per the owner's ruling on that PR.
    NO_DEFAULT is at enum.py:64 with its two comparison sites at :140 and :146.

    A worker is on it. The shape agreed above stands: NO_DEFAULT = object(), with the two == comparisons becoming
    is.

  5. added
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 27, 2026
  6. removed
    wipWork in flight - a covering PR is open or an agent is actively on it
    on Sep 27, 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 valuesenhancementIssues requesting a new capability (set by the feature request template)refactorRestructuring 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