Skip to content

fix(mh): two kept get overrides advertise the base default argument and reject it with TypeError #935

Description

@JarryShaw

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 default

So 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:

  1. Widen both signatures to accept default and delegate, which removes the suppression and makes all seven behave alike. Most invasive, most correct.
  2. Keep the signatures and make the refusal explicit — accept default and raise a clear in-library error saying it is unsupported, so callers get a diagnosis instead of a TypeError.
  3. 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

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

    bugIssues reporting a defect (set by the bug report template; a default, not an assessment)designA design or decision issue: a pattern being decided rather than a defect or a request

    Projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions