Repository navigation
docs(foundation): document the engine members on EngineBase, not Engine (#1024) - #1027
Conversation
JarryShaw
added
docs
…ne (#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 fa3e861, 1237 here. The two `EngineBase.unsupported_reason` references in `docs/source/index.rst` newly resolve; no new miss. tests/project passes.
8ae0d5e to
983fb92
Compare
|
Cross-review verdict on 1. A new
My own fault twice over: I dictated that exact one-line replacement, and my first re-measurement passed the flag string unquoted, which zsh hands over as a single argument, so pylint silently used its default 100-char limit and flagged a pre-existing 107-char line. Re-ran with an array before concluding anything. 2. The commit message's Sphinx count was wrong and self-contradictory. It read 1238, where 1239 − 2 fixed − 0 new = 1237. The body was always right. I supplied the 1238: it was my interim figure on the previous revision, measured before the Also folded in, after measuring it. Deliberately not folded in. Re-measured on |
JarryShaw
added
review: pending
|
Cross-review verdict on First, correcting my own previous comment. I wrote that the new So the defect was adding a fourth to a noisy baseline, not turning a green gate red. It was still real and is gone: Re-verified on
Two method notes worth recording, since both nearly produced false results. The relocation trap has a second layer: normalising only the file line number still leaves docstring-relative offsets, so On |
JarryShaw
added
review: good-to-go
JarryShaw
removed
the
review: good-to-go
`process.rst:104` pinned 157 entries while the file holds 158, so `tests/project` was failing on `main`. * #1020 and #1027 each measured 157 against a 156-entry base, which was correct for each branch alone. Merging both added two entries and left the pin behind -- the parallel-branch collision the page's own "measure, never increment" rule exists to prevent, landing for the first time. Measured rather than incremented: `grep -cE '^\* ' docs/source/changelog/1.5.0.rst` gives 158. tests/project: 268 passed, 1 skipped, 864 subtests passed. The failing test was test_the_page_pins_its_own_measured_numbers.
1 participant
Please follow the guide below
make pylint,make mypy,make isort)make testpasses, and a test case covers the change — not ticked honestly. This is documentation shape, so no test case covers it; the proof is a nitpicky Sphinx build, below.tests/project(268 passed, 864 subtests) andtests/foundation(265 passed, 418 subtests) both pass. The full suite was not run: it needs ~29 GB here and gets OOM-killed.docs/source/changelog/and regeneratedCHANGELOG.mdWhat is the purpose of your pull request?
fix— corrects a defectfeat— adds a featureperf— changes performance, not behaviourrefactor— changes neither behaviour nor performancetest— tests onlydocs— documentation onlyci— workflows or build toolingchore— anything elseDescription of your pull request and other information
Closes #1024. #1023 narrowed
Extractor.engine's return type toEngineBase, but that class was documented under Internal Definitions with:no-members:, so a public property declared a type whose own page exposed no API.This takes the issue's option 1: the member stubs move onto
EngineBase, andEnginekeeps only__init_subclass__— the single member #1023 measured it adds. Option 2, exportingEngineBasein__all__, is deliberately not here; the issue binds that to #1016 as a real API decision.Also folded in: the verbless sentence at
pcapkit/foundation/engines/pcap.py:98-101, and the four_exengdocstring cross-references inextraction.pyrepointed fromEngine.*toEngineBase.*— legal only because option 1 removes the:no-members:obstacle that previously blocked it.Measured with
sphinx -b html -n, becauseconf.pydoes not setnitpickyand the default build is silent about unresolved references. Unresolved references go from 1239 onmainto 1237 here. Two are genuinely fixed:docs/source/index.rst:151and:355referenceEngineBase.unsupported_reason, unresolvable onmainprecisely because of:no-members:. The one apparent new entry is the identical pre-existing annotation warning relocated fromengine.rst:39to:45by the directive move, same target text.One real regression was caught this way and fixed in the same commit:
engine.py:268carried a bare:attr:engine_name``, which resolved againstEnginebefore and missed once the attribute moved. It is now qualified explicitly.