From 983fb92a04d5d56eb800ea7e1a28264cded389f5 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 5 Oct 2026 02:06:40 -0400 Subject: [PATCH] docs(foundation): document the engine members on EngineBase, not Engine (#1024) `Extractor.engine` is declared `EngineBase[_P]` since #1023, but `EngineBase` was documented `:no-members:` under Internal Definitions while `Engine` carried every member stub, so the type a reader followed exposed no API. * `engine.rst`: the member stubs move to `EngineBase`, where they are defined; `Engine` documents only `__init_subclass__` and keeps the customisation `seealso`. `EngineBase` is not added to `__all__` -- that is left to #1016. * Cross-references to `Engine.run`, `read_frame`, `close` and `unsupported_reason` (`pep.rst`, `engines/index.rst`, four `_exeng` docstrings in `extraction.py`) now point at `EngineBase`, where they resolve. * `Engine.__init_subclass__`: qualify the bare `__engine_name__` reference, which stopped resolving once the attribute moved to `EngineBase`. * `PCAP.run`: repair the verbless "will also be save the current `PCAP` engine instance", as #1023 did in `extraction.py`. * Changelog entry added; the entry-count pin in `process.rst` is 157. Also corrected `docs/source/contributing/pep.rst`, which said subclassing an engine registers it automatically. Registration has been opt-in since #514: `__init_subclass__` registers only when the `engine` class keyword is given, and defaults to skipping. Measured -- a subclass without the keyword adds nothing to `Extractor.__engine__`; with it, the name appears. Nitpicky Sphinx build: 1239 unresolved references on fa3e861f5, 1237 here. The two `EngineBase.unsupported_reason` references in `docs/source/index.rst` newly resolve; no new miss. tests/project passes. --- CHANGELOG.md | 1 + docs/source/changelog/1.5.0.rst | 12 ++++++++++++ docs/source/contributing/conventions/process.rst | 2 +- docs/source/contributing/pep.rst | 6 +++--- docs/source/pcapkit/foundation/engines/engine.rst | 11 ++++++----- docs/source/pcapkit/foundation/engines/index.rst | 2 +- pcapkit/foundation/engines/engine.py | 4 +++- pcapkit/foundation/engines/pcap.py | 4 ++-- pcapkit/foundation/extraction.py | 8 ++++---- 9 files changed, 33 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23c29b1e9..db54e0505 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,7 @@ Preceded by `1.5.0a1` (2026-09-15), `1.5.0b1` and `1.5.0b2` (both 2026-09-18) an - subclass registration is **opt-in** for `Engine`, `Reassembly`, `TraceFlow` and `dumpkit`'s `Dumper` ([#514](https://github.com/JarryShaw/PyPCAPKit/issues/514)). Each registers if and only if its registry keyword is given -- `engine=` for `Engine`, `protocol=` for `Reassembly` and `TraceFlow`, `fmt=` for `Dumper`. Previously an absent keyword fell back to the class' own name, so *every* subclass of the public class was registered, and declining meant subclassing the parallel `*Base` class under an alias, which every built-in does (hence the public classes had **0** subclasses against the `*Base` classes' 9, 5, 2 and 3). **This breaks out-of-tree code that subclasses one of the four and relies on the derived key**; pass the keyword, or call the matching `register_*` function. Nothing the library ships is affected, and the `*Base` classes remain importable. Two silent failures are now loud: an unrecognised class keyword raises `UnsupportedCall` instead of being swallowed by `**kwargs` (passing `name=` to a `Reassembly` subclass silently ignored the key, since `protocol=` is the real one), and so does `Dumper`'s `ext=` without `fmt=`. A class attribute is not an opt-in: `__engine_name__` and `__protocol_name__` still set the name a class reports, registered or not. Each metaclass gained a class-level `registry` property mirroring `EnumSchema.registry`, and a `Dumper` subclass no longer touches the filesystem while its `class` statement runs (inferring `fmt` from `kind` meant instantiating it against a `NamedTemporaryFile`). `Engine`'s keyword is `engine=` rather than the `name=` first shipped, because `name` cannot be a class keyword on Python 3.10: `mcls`, `name`, `bases` and `namespace` collide with `abc.ABCMeta.__new__`'s parameters (positional-or-keyword before 3.11, positional-only from 3.11), raising `TypeError` before the hook runs. Those four are the whole collision surface (measured on 3.10.21, 3.11.15 and 3.14.7), and `engine=`, `protocol=` and `fmt=` are outside it. There is no `name=` alias: a keyword that works on some interpreters and not others is the trap being removed. - extraction is around 46% faster on a 1,117-frame HTTP capture, with byte-identical output ([#420](https://github.com/JarryShaw/PyPCAPKit/pull/420)). A reassembled datagram's payload is analysed on first read rather than eagerly, cutting IP reassembly's own cost by 90.7% and TCP's by 23.7% (IP reassembly submits a datagram for every frame, fragmented or not) ([#424](https://github.com/JarryShaw/PyPCAPKit/pull/424)). Flow tracing over the same capture went from 1416.6 ms to 744.0 ms, because the flow dumper handed each record to a `Frame` constructor that re-dissected the whole stack to return the bytes it had just been given; options are no longer parsed twice either ([#427](https://github.com/JarryShaw/PyPCAPKit/pull/427)). - **a breaking change to** `Extractor.engine` and `Extractor.record_header`: both are now declared to return `EngineBase[_P]` rather than `Engine`, and the private `_exeng` attribute follows. Both built-in engines subclass `EngineBase` directly, so neither was ever an `Engine`; the declaration now names the class the object has, and the `cast` calls that hid the gap now launder only the frame type parameter. Nothing is lost: `Engine` adds only the `__init_subclass__` registration hook, and the frame type parameter is preserved. Two things are visible to a type checker. Code passing the result to an `Engine`-typed parameter must now accept `EngineBase`; and because the old declarations were *unparameterised*, `engine.read_frame()` and `record_header().read_frame()` used to be `Any` and are now the extractor's own frame type, so an assignment that relied on `Any` there no longer type-checks ([#1022](https://github.com/JarryShaw/PyPCAPKit/issues/1022)). +- the documentation of the engine base classes: the member stubs (`name`, `module`, `registry`, `extractor`, `unsupported_reason`, `run`, `read_frame`, `close` and the rest) moved from `Engine` to `EngineBase`, where they are defined, and `Engine` now documents only the `__init_subclass__` registration hook it adds. `Extractor.engine` declares `EngineBase[_P]` as its return type, but that class was documented `:no-members:` under *Internal Definitions*, so the type a reader followed led to no API. Cross-references to `Engine.run`, `Engine.read_frame`, `Engine.close` and `Engine.unsupported_reason` now point at `EngineBase`, and the `PCAP` engine's `run` docstring lost the verbless sentence "will also be save the current `PCAP` engine instance". `EngineBase` is still not exported in `__all__`; that is left to [#1016](https://github.com/JarryShaw/PyPCAPKit/issues/1016) ([#1024](https://github.com/JarryShaw/PyPCAPKit/issues/1024)). #### Fixed diff --git a/docs/source/changelog/1.5.0.rst b/docs/source/changelog/1.5.0.rst index baf2f12d4..880f9db59 100644 --- a/docs/source/changelog/1.5.0.rst +++ b/docs/source/changelog/1.5.0.rst @@ -847,6 +847,18 @@ Changed *unparameterised*, ``engine.read_frame()`` and ``record_header().read_frame()`` used to be ``Any`` and are now the extractor's own frame type, so an assignment that relied on ``Any`` there no longer type-checks (:issue:`1022`). +* the documentation of the engine base classes: the member stubs (``name``, + ``module``, ``registry``, ``extractor``, ``unsupported_reason``, ``run``, + ``read_frame``, ``close`` and the rest) moved from ``Engine`` to ``EngineBase``, + where they are defined, and ``Engine`` now documents only the + ``__init_subclass__`` registration hook it adds. ``Extractor.engine`` declares + ``EngineBase[_P]`` as its return type, but that class was documented + ``:no-members:`` under *Internal Definitions*, so the type a reader followed + led to no API. Cross-references to ``Engine.run``, ``Engine.read_frame``, + ``Engine.close`` and ``Engine.unsupported_reason`` now point at ``EngineBase``, + and the ``PCAP`` engine's ``run`` docstring lost the verbless sentence + "will also be save the current ``PCAP`` engine instance". ``EngineBase`` is + still not exported in ``__all__``; that is left to :issue:`1016` (:issue:`1024`). Fixed ~~~~~ diff --git a/docs/source/contributing/conventions/process.rst b/docs/source/contributing/conventions/process.rst index 93b8a39ce..60c4173b7 100644 --- a/docs/source/contributing/conventions/process.rst +++ b/docs/source/contributing/conventions/process.rst @@ -101,7 +101,7 @@ commands down instead of a figure that will be stale by the next merge: The grouping scheme was settled on :issue:`918`: **a section per top-level module, with** ``Added``/``Changed``/``Fixed`` **nested inside each** -- module granularity, not per-file and not per-subpackage. The file carries **9** module-level sections holding -156 entries, and no entry carries an inline kind label:: +157 entries, and no entry carries an inline kind label:: $ grep -cE '^\* \*\*(Added|Changed|Fixed)\*\*' docs/source/changelog/1.5.0.rst 0 diff --git a/docs/source/contributing/pep.rst b/docs/source/contributing/pep.rst index 6071dbf80..14561e874 100644 --- a/docs/source/contributing/pep.rst +++ b/docs/source/contributing/pep.rst @@ -535,9 +535,9 @@ rather than by silently returning nothing -- Adding a further engine does not mean adding handler methods to :class:`~pcapkit.foundation.extraction.Extractor`, as the thread describes. A new engine subclasses :class:`pcapkit.foundation.engines.engine.Engine` and implements just two -methods, :meth:`~pcapkit.foundation.engines.engine.Engine.run` -and :meth:`~pcapkit.foundation.engines.engine.Engine.read_frame`; subclassing -registers it automatically. See :doc:`/ext` for a worked example. What does +methods, :meth:`~pcapkit.foundation.engines.engine.EngineBase.run` +and :meth:`~pcapkit.foundation.engines.engine.EngineBase.read_frame`; passing the +``engine`` class keyword registers it. See :doc:`/ext` for a worked example. What does still apply is the unified auxiliary tools in :mod:`pcapkit.toolkit`, where each engine has a matching module. diff --git a/docs/source/pcapkit/foundation/engines/engine.rst b/docs/source/pcapkit/foundation/engines/engine.rst index 5150635a9..07413f4bf 100644 --- a/docs/source/pcapkit/foundation/engines/engine.rst +++ b/docs/source/pcapkit/foundation/engines/engine.rst @@ -15,6 +15,12 @@ all engine support functionality. For more information on customisation and extension, please refer to :doc:`../../../ext`. + .. automethod:: __init_subclass__ + +.. autoclass:: pcapkit.foundation.engines.engine.EngineBase + :no-members: + :show-inheritance: + .. property:: name :type: str @@ -60,7 +66,6 @@ all engine support functionality. .. automethod:: close .. automethod:: __call__ - .. automethod:: __init_subclass__ .. autoattribute:: __engine_name__ .. autoattribute:: __engine_module__ @@ -68,10 +73,6 @@ all engine support functionality. Internal Definitions -------------------- -.. autoclass:: pcapkit.foundation.engines.engine.EngineBase - :no-members: - :show-inheritance: - .. autoclass:: pcapkit.foundation.engines.engine.EngineMeta :no-members: :show-inheritance: diff --git a/docs/source/pcapkit/foundation/engines/index.rst b/docs/source/pcapkit/foundation/engines/index.rst index d4368279c..750340f6e 100644 --- a/docs/source/pcapkit/foundation/engines/index.rst +++ b/docs/source/pcapkit/foundation/engines/index.rst @@ -138,7 +138,7 @@ so it is worth knowing in advance. Every one of these constraints is also enforced in code rather than only documented: each engine overrides -:meth:`~pcapkit.foundation.engines.engine.Engine.unsupported_reason`, which +:meth:`~pcapkit.foundation.engines.engine.EngineBase.unsupported_reason`, which :meth:`Extractor.run ` consults *before* the import test, so asking for an engine that cannot run here produces one warning naming the actual cause and a clean fall back to the built-in parser. diff --git a/pcapkit/foundation/engines/engine.py b/pcapkit/foundation/engines/engine.py index a13c399e1..f5b576ba1 100644 --- a/pcapkit/foundation/engines/engine.py +++ b/pcapkit/foundation/engines/engine.py @@ -265,7 +265,9 @@ def __init_subclass__(cls, /, engine: 'Optional[str]' = None, *args: 'Any', **kw has guarded on its own ``code`` keyword all along. Note: - :attr:`__engine_name__` is *not* an opt-in. It supplies the + :attr:`__engine_name__ + ` is + *not* an opt-in. It supplies the :attr:`name ` the engine reports, which it does whether or not the engine is registered; only the keyword decides registration. diff --git a/pcapkit/foundation/engines/pcap.py b/pcapkit/foundation/engines/pcap.py index 059ec8e64..3862f8d90 100644 --- a/pcapkit/foundation/engines/pcap.py +++ b/pcapkit/foundation/engines/pcap.py @@ -100,8 +100,8 @@ def run(self) -> 'None': The method will parse the PCAP global header and save the parsed result as :attr:`self.header
`. Information such as PCAP version, data - link layer protocol type, nanosecond flag and byteorder will also be - save the current :class:`PCAP` engine instance. + link layer protocol type, nanosecond flag and byteorder are also saved + on the current :class:`PCAP` engine instance. For output, the method will dump the parsed PCAP global header under the name of ``Global Header``. diff --git a/pcapkit/foundation/extraction.py b/pcapkit/foundation/extraction.py index 58ea481e4..d24b1f199 100644 --- a/pcapkit/foundation/extraction.py +++ b/pcapkit/foundation/extraction.py @@ -770,9 +770,9 @@ def record_header(self) -> 'EngineBase[_P]': def record_frames(self) -> 'None': """Read packet frames. - The method calls :meth:`self._exeng.read_frame ` + The method calls :meth:`self._exeng.read_frame ` to parse each frame from the input PCAP file; and - performs cleanup by calling :meth:`self._exeng.close ` + performs cleanup by calling :meth:`self._exeng.close ` upon completion of the parsing process. Notes: @@ -1111,7 +1111,7 @@ def __iter__(self) -> 'Extractor': def __next__(self) -> '_P': """Iterate and parse next PCAP frame. - It will call :meth:`self._exeng.read_frame ` + It will call :meth:`self._exeng.read_frame ` to parse next PCAP frame internally, until the EOF reached; then it calls :meth:`self._cleanup <_cleanup>` for the aftermath. @@ -1323,7 +1323,7 @@ def _note_eof_progress(self) -> 'bool': def _cleanup(self) -> 'None': """Cleanup after extraction & analysis. - The method calls :meth:`self._exeng.close `, + The method calls :meth:`self._exeng.close `, sets :attr:`self._flag_e ` as :data:`True` and closes the input file *if this class opened it*.