Repository navigation
fix(mh): two kept get overrides advertise the base default argument and reject it with TypeError #935
Description
The two kept @staticmethod get overrides in pcapkit/protocols/internet/mh.py — FastBindingAcknowledgmentStatus and IPv6AddressPrefixCode — now advertise the base's two-argument get(key, default) through inheritance from EnumLookup, and reject it:
FastBindingAcknowledgmentStatus.get('bogus', 'Handover_Accepted')
# TypeError: get() takes 1 positional argument but 2 were given
LMAAddressCode.get('bogus', 'default') # a pure re-parent in the same module
# EnumKeyError: 'bogus' is not a valid LMAAddressCode <- honours defaultSo two of the seven classes #930 re-parented behave differently from the other five on the same call, and the difference is a TypeError rather than a documented refusal. Before the re-parent nothing advertised a default on these classes at all, so this is a gap the re-parenting introduced — not a pre-existing one.
It is exactly what the # type: ignore[override] # pylint: disable=arguments-differ suppression hides. Verified that the suppression is load-bearing: stripping it and re-running mypy yields
mh.py:645:5 error: Signature of "get" incompatible with supertype "EnumLookup" [override]
mh.py:782:5 error: Signature of "get" incompatible with supertype "EnumLookup" [override]
and --warn-unused-ignores reports no unused ignore at either line.
Not a defect in #932, which is why this is its own issue rather than a change there: the behaviour predates that PR's final amendment, it is disclosed verbatim in both docstrings ("no cls, no default … silenced rather than resolved by widening the signature"), and it is orthogonal to the quiet=True change #933 asked for. Filing it so the disclosure does not become the permanent answer.
Three ways out, and the choice is yours:
- Widen both signatures to accept
defaultand delegate, which removes the suppression and makes all seven behave alike. Most invasive, most correct. - Keep the signatures and make the refusal explicit — accept
defaultand raise a clear in-library error saying it is unsupported, so callers get a diagnosis instead of aTypeError. - Leave it, and treat the docstring disclosure plus the suppression as the settled answer.
Surfaced by the Opus cross-review of #932 and verified independently.
Activity
JarryShaw commentedon Sep 29, 2026
I lean on 1.
JarryShaw commentedon Sep 30, 2026
Ruling recorded, verbatim:
So option 1: widen both
getsignatures inpcapkit/protocols/internet/mh.py—FastBindingAcknowledgmentStatusandIPv6AddressPrefixCode— to acceptdefaultand delegate, making all seven re-parented classes behave alike onget(key, default)and removing the two# type: ignore[override] # pylint: disable=arguments-differsuppressions rather than keeping them.That is the most correct of the three and it deletes the defect at its root: the
TypeError: get() takes 1 positional argument but 2 were givengoes away, and the base's contract stops being advertised-but-unhonoured. Verified that the suppressions are load-bearing today — stripping them without widening yields real errors atmh.py:645:5and:782:5,Signature of "get" incompatible with supertype "EnumLookup" [override]— so widening is precisely what lets them go.Blocked on #932, on file contention.
mh.pyis one of that PR's eleven files, and #932 has just gone back toreview: needs-changesover theESPStatusstdlib-IntEnumfinding, so it will be amended again. Two agents editingmh.pyat once lose each other's work silently, which is why this waits rather than racing it.One scoping question, since the two changes now overlap. This fix removes the two suppressions that #932 adds, in the same two functions. Landing #932 with them and then deleting them in a follow-up is churn, and the follow-up would conflict with whatever #932 amends next. Would you rather I fold this into #932 — it is already being amended for the
ESPStatusfinding, and its commit message can carry both issues — or keep it as its own PR after #932 merges? I lean on folding it in, since it touches the same two functions and #932 is not merged. Say if you would rather keep the histories separate.needs: decisionremoved;blockedapplied.refactor(corekit,protocols): normalise the four sentinel objects to S…
refactor(corekit,protocols): normalise the four sentinel objects to S…
fix(protocols): delete the two kept get overrides in mh.py, inherit t…
fix(protocols): delete the two kept get overrides in mh.py, inherit t…
fix(protocols): widen the two kept get overrides in mh.py to accept d…
docs(contributing): record the get-override rulings on the convention…
docs(contributing): record the get-override rulings on the convention…
docs(pcapkit,ci): cite the issue a defect belongs to, not the pull re…
docs(tests): re-point six ruling citations at their actual threads, per
docs(tests): paraphrase four fabricated or altered owner quotations (#…
docs(pcapkit,ci): cite the issue a defect belongs to, not the pull re…
docs(protocols): attribute the get-override ruling to where it was given
docs(tests): paraphrase the six rulings this file still quoted
docs(protocols): state the rulings behind mh, vendor and exceptions i…
docs(tests): paraphrase the six rulings this file still quoted
docs(corekit,tests): state what the enum tiers actually declare and d…
docs(contributing): refer to the owner by role, not by gendered pronoun