From bfac36be41301cb0d5c954274719ee3e8e8de6ef Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sat, 10 Oct 2026 08:32:32 -0400 Subject: [PATCH] refactor(utilities): rename hold_warnings to HoldWarnings (#1599) `hold_warnings`, added in #1588, was the only lowercase class pcapkit defines itself, and needed `# pylint: disable=invalid-name`. Rename it to `HoldWarnings` in `pcapkit/utilities/warnings.py`, its one caller in `pcapkit/protocols/protocol.py`, the two docs pages and #1588's test file, and drop the disable. It has not shipped, so no alias is kept. `HoldWarnings` is documented and imported from another module, so it is public: list it in `pcapkit.utilities.warnings.__all__`, beside `warn`. The `pcapkit_held_warnings` ContextVar and the test's `_HoldNothing` stand-in keep their names: neither embeds the class name. A new test pins that `HoldWarnings` is importable and exported, and that neither `pcapkit.utilities.warnings` nor `pcapkit.protocols.protocol` has a `hold_warnings` attribute. --- docs/source/pcapkit/utilities/functools.rst | 2 +- docs/source/pcapkit/utilities/warnings.rst | 2 +- pcapkit/protocols/protocol.py | 4 +- pcapkit/utilities/warnings.py | 22 +++--- .../test_trial_parse_warnings_1580_unit.py | 70 +++++++++++-------- 5 files changed, 57 insertions(+), 43 deletions(-) diff --git a/docs/source/pcapkit/utilities/functools.rst b/docs/source/pcapkit/utilities/functools.rst index 256447377b..3ff1826b6f 100644 --- a/docs/source/pcapkit/utilities/functools.rst +++ b/docs/source/pcapkit/utilities/functools.rst @@ -43,5 +43,5 @@ Error Handling Utilities .. autofunction:: pcapkit.utilities.warnings.warn -.. autoclass:: pcapkit.utilities.warnings.hold_warnings +.. autoclass:: pcapkit.utilities.warnings.HoldWarnings :members: discard, holding diff --git a/docs/source/pcapkit/utilities/warnings.rst b/docs/source/pcapkit/utilities/warnings.rst index f864c74bf0..01c90f12ec 100644 --- a/docs/source/pcapkit/utilities/warnings.rst +++ b/docs/source/pcapkit/utilities/warnings.rst @@ -30,7 +30,7 @@ class is not an act of reporting one -- it emits nothing and has no side effects A next layer captured shorter than its length hint is parsed on trial, and replaced with :class:`~pcapkit.protocols.misc.raw.Raw` when its header was not captured. The trial's warnings are held back by -:class:`~pcapkit.utilities.warnings.hold_warnings`: a replaced layer's are dropped +:class:`~pcapkit.utilities.warnings.HoldWarnings`: a replaced layer's are dropped on both channels, since they describe a layer the result does not have, and a kept layer's are reported as above when the trial ends (:issue:`1580`). diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index db087f35a5..998dfc7fa1 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -50,7 +50,7 @@ from pcapkit.utilities.exceptions import (BaseError, ProtocolError, ProtocolNotFound, ProtocolNotImplemented, RegistryError, StructError, UnsupportedCall) -from pcapkit.utilities.warnings import RegistryWarning, UnknownFieldWarning, hold_warnings, warn +from pcapkit.utilities.warnings import HoldWarnings, RegistryWarning, UnknownFieldWarning, warn if TYPE_CHECKING: from enum import IntEnum as StdlibEnum @@ -2319,7 +2319,7 @@ def _parse_next_layer(klass: 'Type[ProtocolBase]', file_: 'bytes', length: 'int' """ captured = min(length, len(file_)) - with hold_warnings(_short_of_hint(klass, captured)) as held: + with HoldWarnings(_short_of_hint(klass, captured)) as held: next_ = klass(file_, length, **kwargs) # type: ignore[abstract] length_hint = getattr(next_, '__length_hint__', None) diff --git a/pcapkit/utilities/warnings.py b/pcapkit/utilities/warnings.py index dc282aa583..485423f2aa 100644 --- a/pcapkit/utilities/warnings.py +++ b/pcapkit/utilities/warnings.py @@ -51,7 +51,7 @@ ``pcapkit`` logger is configured separately, e.g. with ``logging.getLogger('pcapkit').setLevel(logging.ERROR)``. -The one exception is a parse :mod:`pcapkit` runs on trial: :class:`hold_warnings` +The one exception is a parse :mod:`pcapkit` runs on trial: :class:`HoldWarnings` holds back its warnings, on both channels, until the parse is kept, and drops them if it is discarded (:issue:`1580`). @@ -70,12 +70,12 @@ from typing_extensions import Self - #: A warning held back by :class:`hold_warnings`: its message, its category, + #: A warning held back by :class:`HoldWarnings`: its message, its category, #: and the frame it is attributed to, if that frame could be found. HeldWarning = Tuple[Union[str, Warning], Type[Warning], Optional[FrameType]] __all__ = [ - 'warn', + 'warn', 'HoldWarnings', # UserWarning 'BaseWarning', @@ -98,11 +98,11 @@ #: :data:`pcapkit.utilities.logging.logger`. logger = get_logger(__name__) -#: The innermost open :class:`hold_warnings` block, or :data:`None` outside any. +#: The innermost open :class:`HoldWarnings` block, or :data:`None` outside any. #: A context variable rather than a global, so that a parse in another thread or #: task is neither held nor released by this one. _HELD = contextvars.ContextVar( - 'pcapkit_held_warnings', default=None) # type: contextvars.ContextVar[Optional[hold_warnings]] + 'pcapkit_held_warnings', default=None) # type: contextvars.ContextVar[Optional[HoldWarnings]] def warn(message: 'Union[str, Warning]', category: 'Type[Warning]', @@ -132,7 +132,7 @@ def warn(message: 'Union[str, Warning]', category: 'Type[Warning]', how to silence either channel. Note: - Inside a :class:`hold_warnings` block the warning is held back instead, + Inside a :class:`HoldWarnings` block the warning is held back instead, with the frame it is attributed to, and reported from there only if the block keeps it. @@ -167,7 +167,7 @@ def warn(message: 'Union[str, Warning]', category: 'Type[Warning]', warnings.warn(message, category, stacklevel) -def _open_block(block: 'Optional[hold_warnings]') -> 'Optional[hold_warnings]': +def _open_block(block: 'Optional[HoldWarnings]') -> 'Optional[HoldWarnings]': """The innermost open block from ``block`` outwards. Args: @@ -186,7 +186,7 @@ def _open_block(block: 'Optional[hold_warnings]') -> 'Optional[hold_warnings]': def _replay(message: 'Union[str, Warning]', category: 'Type[Warning]', target: 'Optional[FrameType]') -> 'None': - """Report a warning :class:`hold_warnings` held back, as :func:`warn` would have. + """Report a warning :class:`HoldWarnings` held back, as :func:`warn` would have. Args: message: Warning message. @@ -214,7 +214,7 @@ def _replay(message: 'Union[str, Warning]', category: 'Type[Warning]', warnings.warn(message, category, level) -class hold_warnings: # pylint: disable=invalid-name +class HoldWarnings: """Hold back the warnings a parse reports until it is known to be kept. Args: @@ -268,8 +268,8 @@ def __init__(self, hold: 'bool' = True) -> 'None': self.holding = hold self._held = [] # type: List[HeldWarning] - self._outer = None # type: Optional[hold_warnings] - self._token = None # type: Optional[contextvars.Token[Optional[hold_warnings]]] + self._outer = None # type: Optional[HoldWarnings] + self._token = None # type: Optional[contextvars.Token[Optional[HoldWarnings]]] self._open = False def __enter__(self) -> 'Self': diff --git a/tests/protocols/test_trial_parse_warnings_1580_unit.py b/tests/protocols/test_trial_parse_warnings_1580_unit.py index 4caf9552b4..9ac1d0d449 100644 --- a/tests/protocols/test_trial_parse_warnings_1580_unit.py +++ b/tests/protocols/test_trial_parse_warnings_1580_unit.py @@ -12,7 +12,7 @@ the same, in either byte order. A next layer captured shorter than its length hint is now parsed under -:class:`~pcapkit.utilities.warnings.hold_warnings`, and its warnings are dropped +:class:`~pcapkit.utilities.warnings.HoldWarnings`, and its warnings are dropped with the layer. A layer that is kept reports them from the caller's line when its trial ends, inside the same guard -- so under an ``error`` filter every chain is the one parsed holding nothing -- and a cut header that stays in the chain still @@ -74,7 +74,7 @@ class _HoldNothing: - """A stand-in for :class:`hold_warnings` that holds nothing, as before #1580.""" + """A stand-in for :class:`HoldWarnings` that holds nothing, as before #1580.""" discard = False @@ -227,24 +227,24 @@ def test_holds_nest(self) -> None: from pcapkit.protocols.link.ethernet import Ethernet from pcapkit.protocols.misc.raw import Raw from pcapkit.protocols.protocol import ProtocolBase - from pcapkit.utilities.warnings import SchemaWarning, hold_warnings, warn + from pcapkit.utilities.warnings import HoldWarnings, SchemaWarning, warn with warnings.catch_warnings(record=True) as caught: warnings.simplefilter('always') - with hold_warnings(): + with HoldWarnings(): warn('outer, before', SchemaWarning) - with hold_warnings() as inner: + with HoldWarnings() as inner: warn('inner, discarded', SchemaWarning) inner.discard = True trial = ProtocolBase._parse_next_layer(Ethernet, bytes(10), 10) # pylint: disable=protected-access - with hold_warnings(): + with HoldWarnings(): warn('inner, kept', SchemaWarning) - with hold_warnings(False): + with HoldWarnings(False): warn('held by nothing', SchemaWarning) self.assertEqual(self._schema(caught), ['inner, kept', 'held by nothing']) warn('outer, after', SchemaWarning) - with hold_warnings() as outer: - with hold_warnings(): + with HoldWarnings() as outer: + with HoldWarnings(): warn('kept where the inner block is left', SchemaWarning) warn('dropped with the outer block', SchemaWarning) outer.discard = True @@ -258,7 +258,7 @@ def test_layer_that_cannot_be_replaced_holds_nothing(self) -> None: from pcapkit.protocols.link.ethernet import Ethernet from pcapkit.protocols.misc.raw import Raw from pcapkit.protocols.protocol import ProtocolBase, _short_of_hint - from pcapkit.utilities.warnings import hold_warnings + from pcapkit.utilities.warnings import HoldWarnings class NeedsAnInstance: """A hint that cannot be read from the class.""" @@ -273,7 +273,7 @@ def __length_hint__(self) -> 'int': frame = MACS + b'\x08\x00' + IPV4_TCP[:18] with warnings.catch_warnings(record=True) as caught: warnings.simplefilter('always') - with hold_warnings() as outer: + with HoldWarnings() as outer: parsed = ProtocolBase._parse_next_layer(Ethernet, frame, len(frame)) # pylint: disable=protected-access outer.discard = True self.assertEqual(str(parsed.protochain), 'Ethernet:Internet_Protocol_version_4') @@ -304,7 +304,7 @@ def chains() -> 'list[str]': return found held = chains() - with unittest.mock.patch.object(protocol_module, 'hold_warnings', _HoldNothing): + with unittest.mock.patch.object(protocol_module, 'HoldWarnings', _HoldNothing): reference = chains() self.assertEqual(len(held), sum(map(len, FRAMES))) self.assertEqual(held, reference) @@ -335,7 +335,7 @@ def reports() -> 'list[list[tuple[Any, ...]]]': return found held = reports() - with unittest.mock.patch.object(protocol_module, 'hold_warnings', _HoldNothing): + with unittest.mock.patch.object(protocol_module, 'HoldWarnings', _HoldNothing): reference = reports() for kept, every in zip(held, reference): remaining = iter(every) @@ -344,7 +344,7 @@ def reports() -> 'list[list[tuple[Any, ...]]]': def test_base_exception_is_let_through(self) -> None: """A :exc:`BaseException` leaves a block as itself, its held warnings dropped.""" - from pcapkit.utilities.warnings import SchemaWarning, hold_warnings, warn + from pcapkit.utilities.warnings import HoldWarnings, SchemaWarning, warn for error in (KeyboardInterrupt, SystemExit, GeneratorExit): for action in ('error', 'always'): @@ -352,19 +352,19 @@ def test_base_exception_is_let_through(self) -> None: with warnings.catch_warnings(record=True) as caught: warnings.simplefilter(action) with self.assertRaises(error): - with hold_warnings(): + with HoldWarnings(): warn('held when the interrupt came', SchemaWarning) raise error() self.assertEqual(caught, []) def test_hold_left_by_an_error_reports(self) -> None: """A block left by an exception reports what it held, discarded or not.""" - from pcapkit.utilities.warnings import SchemaWarning, hold_warnings, warn + from pcapkit.utilities.warnings import HoldWarnings, SchemaWarning, warn with warnings.catch_warnings(record=True) as caught: warnings.simplefilter('always') with self.assertRaises(KeyError): - with hold_warnings() as held: + with HoldWarnings() as held: warn('held, then an error', SchemaWarning) held.discard = True raise KeyError('mid-parse') @@ -372,22 +372,22 @@ def test_hold_left_by_an_error_reports(self) -> None: def test_held_frame_gone_is_reported_outside_pcapkit(self) -> None: """A level past the stack, with no frame to find again, still blames this file.""" - from pcapkit.utilities.warnings import SchemaWarning, hold_warnings, warn + from pcapkit.utilities.warnings import HoldWarnings, SchemaWarning, warn with warnings.catch_warnings(record=True) as caught: warnings.simplefilter('always') - with hold_warnings(): + with HoldWarnings(): warn('past the outermost frame', SchemaWarning, stacklevel=10 ** 6) self.assertEqual([(str(item.message), item.filename) for item in caught], [('past the outermost frame', __file__)]) def test_hold_is_local_to_its_context(self) -> None: """A warning reported in another context is not held by this one's block.""" - from pcapkit.utilities.warnings import SchemaWarning, hold_warnings, warn + from pcapkit.utilities.warnings import HoldWarnings, SchemaWarning, warn with warnings.catch_warnings(record=True) as caught: warnings.simplefilter('always') - with hold_warnings() as held: + with HoldWarnings() as held: contextvars.Context().run(warn, 'another context', SchemaWarning) self.assertEqual(self._schema(caught), ['another context']) held.discard = True @@ -396,10 +396,10 @@ def test_hold_is_local_to_its_context(self) -> None: def test_hold_left_out_of_order(self) -> None: """A generator's block left out of order neither ends another block nor outlives itself.""" import pcapkit.utilities.warnings as warnings_module - from pcapkit.utilities.warnings import SchemaWarning, hold_warnings, warn + from pcapkit.utilities.warnings import HoldWarnings, SchemaWarning, warn def generator() -> 'Any': - with hold_warnings(): + with HoldWarnings(): warn('generator', SchemaWarning) yield @@ -409,14 +409,14 @@ def generator() -> 'Any': # the generator's block is left while the caller's, opened after it, is open suspended = generator() next(suspended) - with hold_warnings() as caller: + with HoldWarnings() as caller: next(suspended, None) warn('caller, discarded', SchemaWarning) caller.discard = True warn('after both', SchemaWarning) # the caller's block is left while the generator's, opened inside it, is open - with hold_warnings(): + with HoldWarnings(): suspended = generator() next(suspended) next(suspended, None) @@ -427,12 +427,12 @@ def generator() -> 'Any': def test_hold_left_in_a_copied_context(self) -> None: """A block left in a copy of its context unsets itself there, and is stepped over here.""" import pcapkit.utilities.warnings as warnings_module - from pcapkit.utilities.warnings import SchemaWarning, hold_warnings, warn + from pcapkit.utilities.warnings import HoldWarnings, SchemaWarning, warn with warnings.catch_warnings(record=True) as caught: warnings.simplefilter('always') - with hold_warnings() as outer: - block = hold_warnings() + with HoldWarnings() as outer: + block = HoldWarnings() block.__enter__() # pylint: disable=unnecessary-dunder-call contextvars.copy_context().run(block.__exit__, None, None, None) warn('held by the outer block', SchemaWarning) @@ -468,6 +468,20 @@ def test_filters_and_registries_are_untouched(self) -> None: self.assertEqual(len(unrelated), 1) self.assertEqual(self._schema(caught), TCP_CUT_MESSAGES) + def test_hold_class_is_named_in_camel_case(self) -> None: + """The class is ``HoldWarnings`` (#1599); it never shipped lowercase, so no alias is kept.""" + import pcapkit.protocols.protocol as protocol_module + import pcapkit.utilities.warnings as warnings_module + from pcapkit.utilities.warnings import HoldWarnings + + self.assertEqual(HoldWarnings.__name__, 'HoldWarnings') + self.assertIs(protocol_module.HoldWarnings, HoldWarnings) + self.assertIn('HoldWarnings', warnings_module.__all__) + self.assertNotIn('hold_warnings', warnings_module.__all__) + for module in (warnings_module, protocol_module): + with self.subTest(module=module.__name__): + self.assertFalse(hasattr(module, 'hold_warnings')) + if __name__ == '__main__': unittest.main()