Skip to content

three names used in string annotations are never imported, one of them raising NameError under get_type_hints; and MANIFEST.in now contradicts itself about what an sdist ships #642

Description

@JarryShaw

Five textual defects, grouped because each is a one-line correction with no behavioural change and no regression test to write — a fixer would do them in one pass. They are independent of each other; the sections below are separately actionable.

Three are names used in string annotations that the module never imports. Two are documentation claims that the repository's own later measurements contradict.

All measured on 375e9d411, CPython 3.14.7, mypy 2.3.1.


1. Three names used in string annotations are never imported

$ mypy pcapkit
pcapkit/utilities/logging.py:350:54: error: Name "Any" is not defined  [name-defined]
pcapkit/protocols/schema/internet/ipv6_route.py:136:77: error: Name "Protocol" is not defined  [name-defined]
pcapkit/protocols/schema/internet/ipv6_route.py:271:24: error: Name "Optional" is not defined  [name-defined]
Found 115 errors in 39 files (checked 496 source files)

Those three are the complete set of name-defined findings in the package — grep -c name-defined over the full run returns exactly 3. (The other 112 errors are unrelated categories and are not this issue.)

The three are not equally harmless, and the difference is the point.

pcapkit/utilities/logging.py:350 — Any, and this one actually raises

def _writes_to(candidate: 'logging.Handler', stream: 'Any') -> 'bool':

The module's TYPE_CHECKING block is logging.py:45-46:

if TYPE_CHECKING:
    from typing import IO, Optional, Union

No Any. Line 350 is the only occurrence of Any in the file.

_writes_to is a plain module-level function, defined at runtime, so its annotations are reachable:

MEASURED TREE: .../pcapkit/__init__.py
--- logging._writes_to ---
  NameError: name 'Any' is not defined

that is typing.get_type_hints(pcapkit.utilities.logging._writes_to). So this is a live failure, not a latent one — any tool that resolves annotations on this function (a docs generator, a runtime validator, attrs/pydantic-style introspection, inspect.get_annotations(..., eval_str=True)) hits NameError. Adding Any to line 46 fixes it.

pcapkit/protocols/schema/internet/ipv6_route.py:136 — Protocol, latent by construction

    if TYPE_CHECKING:
        def __init__(self, next: 'Enum_TransType', length: 'int', type: 'Enum_Routing',
                     seg_left: 'int', data: 'bytes | RoutingType', payload: 'Protocol | Schema | bytes') -> 'None': ...

Schema is imported (:15). Protocol is not — its only other occurrences in the file are :245-246, inside a comment. The TYPE_CHECKING block is :25-29 and imports IPv6Address, Any and FieldBase as Field.

This one cannot fail at runtime, and I checked rather than assuming: the stub is never the runtime __init__.

init: <function IPv6_Route.__init__ at 0x7f697909c670>
defined at: ('<string>', 2)
raw __annotations__: {}
get_type_hints(init) -> {}

The runtime __init__ is generated from <string> by the schema machinery and carries no annotations at all, so the 'Protocol | Schema | bytes' string is only ever read by a type checker. get_type_hints on the class itself succeeds. So this is a type-checker-visible defect only — but it is still wrong, and it misleads a reader into thinking Protocol is in scope.

pcapkit/protocols/schema/internet/ipv6_route.py:271 — Optional, latent, and next to an unreachable

        dst_val = cast('Optional[IPv6Address]', packet.get('dst'))

:271 is the only occurrence of Optional in the file. typing.cast never evaluates a string first argument, confirmed directly:

--- cast() with unresolvable string: does it raise? ---
  cast -> 42

(typing.cast('Optional[Nonexistent]', 42) returns 42.) So latent, as with the one above.

Worth fixing together with its neighbour, because mypy flags dead code in the same function:

pcapkit/protocols/schema/internet/ipv6_route.py:264:13: error: Statement is unreachable  [unreachable]

:264 starts a block that ends return self at :269 — two lines above :271. Whether that block should be reachable is a separate question from the missing import, and I have not investigated it; flagging it here only because anyone editing :271 will be looking straight at it.


2. MANIFEST.in:14-17 states something the repository has since measured false — and now contradicts itself seven lines later

MANIFEST.in:14-19:

14	# Load-bearing, not belt-and-braces. ``global-include *.rst`` above matches only
15	# ``*.rst``, so since the README became Markdown this line is the only thing that
16	# puts it in an sdist -- and ``setup.py`` reads it unguarded for the
17	# ``long_description``, so an sdist without it cannot be installed at all.
18	include README.md
19	include LICENSE

Both sentences are false. Re-measured for this issue.

"the only thing that puts it in an sdist": deleting lines 14-19 and rebuilding from a clean tree (git archive HEAD into a scratch directory, rm -rf dist build *.egg-info, python -m build --sdist) produces a sdist whose tar tzf listing is byte-identical to the baseline — 861 entries both ways, diff of the two listings empty. README.md, LICENSE, CHANGELOG.md and CITATION.cff are all still present. The build log shows copying README.md -> pypcapkit-1.5.0b4/ and, twice, adding license file 'LICENSE'.

"cannot be installed at all": installing that same sdist succeeds.

$ /tmp/manifest-venv/bin/pip install --no-deps /tmp/manifest-exp/dist/pypcapkit-1.5.0b4.tar.gz
Building wheel for pypcapkit (pyproject.toml): finished with status 'done'
Successfully installed pypcapkit-1.5.0b4

Exit 0, no error of any kind. (--no-deps only to skip the dependency tree; a subsequent import pcapkit fails on the missing tbtrim runtime dep, which is an artefact of --no-deps and not evidence either way.)

Why both files ship regardless:

  • setuptools/command/sdist.py — sdist.READMES = ('README', 'README.rst', 'README.txt', 'README.md'). Setuptools' sdist adds any of these unconditionally, independent of MANIFEST.in. Verified live on setuptools 84.0.0.
  • setuptools/dist.py:460 — the default license_files glob ['LICEN[CS]E*', 'COPYING*', 'NOTICE*', 'AUTHORS*'], used because pyproject.toml sets only license = { text=... } and declares no license-files. setuptools/command/egg_info.py:627-631 is what emits adding license file 'LICENSE'.
  • setup.py:33-37 — get_long_description() does read the file unguarded, so that half of the comment is accurate about the code. But it reads from the working tree during the build, not from an unpacked sdist, so its failure mode is a checkout missing README.md, not a published sdist missing it.

The part that makes this more than a stale comment

#631 measured exactly this and wrote the correct version into the same file, seven lines below, without correcting the wrong one. MANIFEST.in:25-26:

25	# there a default to fall back on: setuptools ships ``LICENSE`` from its
26	# ``license_files`` whether or not this file mentions it, but no packaging

So MANIFEST.in now asserts both that include LICENSE is "load-bearing" (:14) and that setuptools ships LICENSE "whether or not this file mentions it" (:25-26). git show on #631's commit confirms it was a pure insertion — 16 lines added after include LICENSE, zero deletions — so lines 14-17 survive verbatim from before, merely renumbered.

The same contradiction was reproduced into the changelogs. docs/source/changelog/1.5.0.rst:961-965, in the #619 entry:

  ``text/markdown``, and the ``include README.md`` line in ``MANIFEST.in`` is now
  the only thing that puts the README in a source distribution, because
  ``global-include *.rst`` no longer matches it -- which matters, since
  ``setup.py`` reads the file unguarded and an sdist without it cannot be
  installed.

against docs/source/changelog/1.5.0.rst:1110-1114, in the #631 entry 150 lines below:

  there a default to fall back on. Removing all three of those ``include`` lines and
  rebuilding shows ``README.md`` and ``LICENSE`` shipping anyway, setuptools adding
  the latter from ``license_files`` and recording it as ``License-File: LICENSE`` in
  ``PKG-INFO``, while ``CHANGELOG.md`` vanishes -- so of the three only
  ``CHANGELOG.md`` is load-bearing

and the identical pair exists in CHANGELOG.md:75 (claim) and CHANGELOG.md:85 (contradiction).

So the false claim stands in three places: MANIFEST.in:14-17, docs/source/changelog/1.5.0.rst:961-965, CHANGELOG.md:75. A published changelog that asserts P and not-P about the same mechanism is worse than one that is merely out of date, because a reader cannot tell which entry to trust.

Historical entries are normally left alone as a record of what was believed at the time. That convention does not cover this case: the #619 entry is not describing a belief that later changed, it is stating a packaging fact that was never true. There is precedent for correcting one in place — the entry at 1.5.0.rst:1135-1169 exists precisely to fix "two inaccurate claims in the #630 entry above".

Factually, the comment should say that include README.md and include LICENSE are belt-and-braces rather than load-bearing, naming sdist.READMES and the default license_files glob as what actually ships them, and that of the original three include lines only CHANGELOG.md is load-bearing.


3. The #619 changelog entry cites no issue number

$ grep -cE "\(#619\)" docs/source/changelog/1.5.0.rst
0
$ grep -cE "\(#619\)" CHANGELOG.md
0

The only #619 anywhere in docs/source/changelog/1.5.0.rst is :1121, which is #631's entry referring back to it ("#619 did not address it") — a reference, not a citation.

The entry itself is docs/source/changelog/1.5.0.rst:944-979, the * **Changed** -- the README is a landing page now, ... bullet. It ends at :979 with not the project's. and no citation. Its immediate neighbours both carry one: the bullet at :929-943 ends (#603), and the one at :980-1006 ends (#624).

Measured base rate over the file: 71 of 76 bullets carry a parenthesised (#nnn) citation; 5 do not, and the #619 entry is one of the 5. The other four are at :136-139, :181-214, :336-338 and :343-350 — not investigated here, and possibly deliberate, but listed so the fix is not mistaken for the only gap.

Same omission in CHANGELOG.md:75, the corresponding bullet there.

Reported, not verified by me: that the cause was a web-UI "Update branch" merge reordering the bullets and dropping the trailer. That is plausible and matches the entry sitting out of numeric order among its neighbours, but I did not reconstruct the merge to confirm it.


Notes

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

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions