Skip to content

fix(corekit,utilities): raise pcapkit exceptions from EnumLookup.get, following stdlib Enum's shape #923

Description

@JarryShaw

The maintainer's ruling on #921, verbatim:

Either ValueError or KeyError, that's depending on how stdlib's Enum would raise on these circumstances. And we should raise one from pcapkit.utilities.exceptions rather builtin exceptions.

What stdlib does, measured on Python 3.14.7:

call stdlib Enum
E['nosuch'] — name miss KeyError: 'nosuch'
E(999) — value miss ValueError: 999 is not a valid E
E(None) ValueError: None is not a valid E

So the shape the ruling selects is: a name miss is KeyError-derived, a value miss is ValueError-derived — and both must come from pcapkit.utilities.exceptions, not from builtins.

Where the library stands against that. EnumLookup.get (pcapkit/corekit/enum.py:395) surfaces a bare builtin KeyError on a name miss and a bare builtin ValueError on a value miss, so it has the right shape and the wrong provenance. Two classes work around it by converting: TransportProtocol.get and Criticality.get both catch the base's KeyError and re-raise ValueError naming the key, which under this ruling is the wrong direction — a name miss should stay KeyError-shaped.

A gap that has to be filled first. pcapkit/utilities/exceptions.py already has the value half: EnumValueError(BaseError, ValueError) at line 374. It has no KeyError-derived class at all — every member of that module derives from TypeError, AttributeError, ValueError, IOError, IndexError, FileExistsError or FileNotFoundError. So the name-miss half needs a new exception, something like EnumKeyError(BaseError, KeyError), before get can comply.

Scope

  1. Add the missing KeyError-derived exception to pcapkit/utilities/exceptions.py and its __all__.
  2. Make EnumLookup.get raise the two pcapkit.utilities.exceptions classes instead of builtins, keeping the stdlib shape — name miss KeyError-derived, value miss ValueError-derived. Both stay catchable as the builtins they derive from, so existing except KeyError / except ValueError call sites keep working.
  3. Remove the now-wrong conversion from TransportProtocol.get and Criticality.get. Criticality.get disappears entirely at that point; TransportProtocol.get reduces to the .lower() call and nothing else.
  4. Update the docstrings and docs/source/contributing/conventions.rst's registry-protocol section, which documents the current exception contract.

Blast radius is real and not yet fully measured. This changes the contract for every concrete EnumLookup subclass. A census is running to establish how many already surface ValueError versus KeyError on a name miss and what in pcapkit/ and tests/ catches KeyError around a .get( call; I will post the counts here when it lands.

Blocked on #921 and #922 merging. Both touch pcapkit/corekit/enum.py or its subclasses and both are review: good-to-go; starting here first would collide with them.

Activity

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

    breakingBreaks public-facing behaviour or API (apply alongside the type label)fixPull requests that fix a defect (fix: subject prefix)

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions