From 84e5035d3dd7a9776434d13e7adf130874241b4e Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 2 Oct 2026 17:17:29 -0400 Subject: [PATCH] docs(corekit,tests): state what the enum tiers actually declare and dispatch Four prose defects under #719, each verified against the code rather than read off the existing text. Prose only: both files are token-identical to main with strings masked. - enum.py said the const registries "inherit them from here", of all four of get/get_all/register/register_alias. Only two are declared here: vars(EnumLookup) owns get and get_all, vars(EnumRegistry) owns register and register_alias, so the claim was transitively true at best. The prose now says which tier declares which, and that a registry reaches all four from this module. - enum.py said AppType overrides all four "to route through its _dispatch". Two of the four route through it. An AST walk of AppType's own methods finds a _dispatch call in get and get_all only; register mentions it in a docstring that explains why it deliberately does not dispatch, and apptype.py:2766-2771 gives the reason -- minting must never leak onto a transport IANA never assigned the service to. The old prose asserted the opposite of a documented design decision. The same claim one paragraph down is narrowed from "tier 2's methods" to "tier 2's lookups", since the premise is _dispatch's return value. - The tier-2 sentence covered register and register_alias with "minting", which is exact for the first and loose for the second: register_alias registers an alias rather than minting a member, and its own docstring frames the concern as a registration. It now says "a write", naming both. - Two docstring lines had drifted to 91 columns inside paragraphs otherwise wrapped at 74-82. Reflowed to their own neighbours' band; no word changed. - test_enum_lookup_reparent_930_unit.py claimed no call site in the tree had ever passed get a key of a third type. That is not statically decidable -- a key arriving through a variable is invisible to any search -- so the docstring now states what the change did establish and says why the stronger form cannot be. No citation moved, and the issue numbers left standing were each checked against the API: #842, #860, #877 and #935 are all issues. tests/corekit/{test_enum_lookup_reparent_930,test_enum_lookup_base, test_enum_lookup_reparent_877,test_enum_get_exception_provenance_923}_unit.py and tests/project/test_conventions_doc_claims.py pass: 167 passed, 1 skipped, 252 subtests, exit code 0 read from the process. --- pcapkit/corekit/enum.py | 52 ++++++++++--------- .../test_enum_lookup_reparent_930_unit.py | 12 ++--- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/pcapkit/corekit/enum.py b/pcapkit/corekit/enum.py index 33a5dabb7..09d7eb17c 100644 --- a/pcapkit/corekit/enum.py +++ b/pcapkit/corekit/enum.py @@ -64,21 +64,25 @@ That is a three-tier hierarchy, of which this module is **tier one**: 1. :class:`EnumRegistry` -- the four methods, in the form that suits a registry - mapping one key to one member. The registries under :mod:`pcapkit.const` - inherit them from here, apart from the overrides below and a few hand-written - ``get`` overrides. -2. ``AppType``'s sub-base -- overrides all four to route through its - ``_dispatch``, because a port lookup needs a transport protocol to be - answerable at all. Landed as of GitHub issue #860: not in this module, but - in :class:`pcapkit.const.reg.apptype.apptype.AppType` itself, which now - mixes in :class:`EnumRegistry` directly and overrides ``get``, ``get_all``, - ``register`` and ``register_alias`` with that dispatch, plus - ``_unregistered_member`` for its own three extra attributes (``svc``, + mapping one key to one member. Two are its own, ``register`` and + ``register_alias``; ``get`` and ``get_all`` are declared one level up on + :class:`EnumLookup` and reach it by inheritance. So a registry under + :mod:`pcapkit.const` gets all four from this module, apart from the + overrides below and a few hand-written ``get`` overrides. +2. ``AppType``'s sub-base -- overrides all four, routing the two lookups + through its ``_dispatch``, because a port lookup needs a transport protocol + to be answerable at all. Landed as of GitHub issue #860: not in this module, + but in :class:`pcapkit.const.reg.apptype.apptype.AppType` itself, which + mixes in :class:`EnumRegistry` directly. ``register`` and ``register_alias`` + are overridden without that dispatch, staying on the registry they are + called on, so that a write never leaks onto a transport IANA never assigned + the service to -- a minted member for the first, an alias for the second. + Plus ``_unregistered_member``, for its own three extra attributes (``svc``, ``port``, ``proto``) that the generic one below does not know to set. 3. The ``AppType`` transport subclasses -- ``TCP``, ``UDP``, ``SCTP``, ``DCCP`` -- turned out to need no override of their own at all: ``_dispatch`` already returns ``cls`` unchanged the moment ``cls.__registry__`` is not - :obj:`None`, which is true for exactly these four, so tier 2's methods + :obj:`None`, which is true for exactly these four, so tier 2's lookups already answer correctly on each of them without a further layer. Before this, the four methods lived as generated *text*: written out longhand in @@ -164,11 +168,11 @@ class EnumLookup: def _validate_value(cls, value: 'Any') -> 'None': """Hook: reject ``value`` if this enumeration's contract does not allow it. - GitHub issue #877 requires some range-validation logic for the inheriting - classes to hook into. This is that hook, and it is what the bare tier carries - **instead** of ``register``: what values are *legal* is something every enumeration - has an opinion on, whereas who may *add* one is only an open registry's - concern. + GitHub issue #877 requires some range-validation logic for the + inheriting classes to hook into. This is that hook, and it is what the + bare tier carries **instead** of ``register``: what values are *legal* + is something every enumeration has an opinion on, whereas who may *add* + one is only an open registry's concern. The base implementation accepts everything, because a base cannot know any subclass's range. Overriding it is how a subclass states one -- the @@ -260,14 +264,14 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': out: an unrecognised or unregistered value does not become a registered member unless a user or caller explicitly creates one. A value inside a registry's declared-but-unassigned range still resolves, through that - registry's own ``_missing_`` and :meth:`_unregistered_member`, to a member - that is deliberately absent from the lookup tables -- true outside the one registry - named above, where such a value instead lands in *both* tables, - exactly as :meth:`register` would leave it -- for a non-``str`` key; - the ``str`` case is qualified below. Both describe ``key`` resolution - only. ``default`` never reaches ``_missing_`` on either branch: a - declared-but-unassigned ``default`` does not resolve to an - unregistered member the way such a ``key`` does -- it simply does + registry's own ``_missing_`` and :meth:`_unregistered_member`, to a + member that is deliberately absent from the lookup tables -- true + outside the one registry named above, where such a value instead lands + in *both* tables, exactly as :meth:`register` would leave it -- for a + non-``str`` key; the ``str`` case is qualified below. Both describe + ``key`` resolution only. ``default`` never reaches ``_missing_`` on + either branch: a declared-but-unassigned ``default`` does not resolve + to an unregistered member the way such a ``key`` does -- it simply does not resolve, and the lookup error ``key`` itself would have raised propagates instead. diff --git a/tests/corekit/test_enum_lookup_reparent_930_unit.py b/tests/corekit/test_enum_lookup_reparent_930_unit.py index b111ae319..8ac5c77b8 100644 --- a/tests/corekit/test_enum_lookup_reparent_930_unit.py +++ b/tests/corekit/test_enum_lookup_reparent_930_unit.py @@ -631,12 +631,12 @@ class NonCanonicalKeyConvergenceTests(unittest.TestCase): out to make true, and the two overrides are exactly what kept it from being true before this. - Verified before writing this test: no call site in this tree -- tests - included -- ever passed ``get`` a key that is neither an :class:`int` - nor a :class:`str`, so this divergence was live on the two overrides - but never actually reached; its removal changes no behaviour any - caller in this tree observed, only what a caller passing such a key - would see. + Scope of that change, stated as narrowly as the evidence supports: + what moved is the exception a key of some third type raises. Whether + any caller ever passed one was not established -- a key reaching + ``get`` through a variable cannot be read off a search of the tree -- + so this test pins the converged behaviour directly rather than resting + on the divergence having been unreachable. """ def test_none_and_float_keys_all_raise_enumvalueerror(self) -> None: