Skip to content

fix(const): give the AppType registry dict union only where dict has it (#1694) - #1703

Merged
JarryShaw merged 1 commit into
mainfrom
fix/1694-apptype-dict-union-py38
Oct 14, 2026
Merged

JarryShaw merged 1 commit into
mainfrom
fix/1694-apptype-dict-union-py38

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly

  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])

  • Use Preview tab to see how your pull request will actually look like

  • Searched for similar pull requests

  • Followed the coding style (make pylint, make mypy, make isort)

  • make test passes, and a test case covers the change. Only the touched module was run, on 3.8, 3.9, 3.10 and 3.14. The full suite is left to CI.

  • Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible: N/A — centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

Tick the commit type your subject line carries.

  • fix — corrects a defect
  • feat — adds a feature
  • perf — changes performance, not behaviour
  • refactor — changes neither behaviour nor performance
  • test — tests only
  • docs — documentation only
  • ci — workflows or build tooling
  • release — bumps the version or rolls up a distribution
  • chore — anything else

Description of your pull request and other information

_Registries.__or__/__ror__ called dict.__or__/dict.__ror__ without a check. Dict union is new in 3.9 (PEP 584). On 3.8, any | or |= with AppType.__registries__ imported TCP and UDP and then raised AttributeError, where a plain dict raises TypeError.

  • Fix: both methods are now defined only if sys.version_info >= (3, 9), a version gate in the style of pcapkit/utilities/compat.py. __dir__ leaves out the new sys import, as it already does importlib. On 3.8, all four forms (r | {}, {} | r, r |= {}, {} |= r) now raise dict's TypeError, naming '_Registries', and import nothing. A 26-case probe of union forms (operator, dunder and hasattr calls) gives the same output before and after on 3.9, 3.10 and 3.14. |= keeps dict's own in-place update, which reads no value, so the registry stays pending.
  • Tests: the or/ror views run from 3.9 only, and an assertion pins which views ran. The new test_union_is_dict_s_on_every_python compares the four forms with dict's behaviour on the running interpreter.
  • Fail-before: on origin/main under 3.8.20, the two tests named in the issue fail with AttributeError (2 failed), and the new test errors in all four subtests. On 3.9.25, main passes 14 of 14.
  • After: 15 passed on each of 3.8.20, 3.9.25, 3.10.21 and 3.14.8. Line and branch coverage of apptype/__init__.py is 100% on 3.14. pylint, mypy and isort report nothing new.
  • Other overrides: every other method the registry overrides already exists on 3.8's dict. The 3.10+ keys().mapping reads go through the registry's own methods.

Closes #1694

@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 14, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 6be6781e4. A Sonnet cross-review reproduced #1694 on 3.8 at base: AttributeError: type object 'dict' has no attribute '__or__'. On this head the module passes 15 tests on 3.8, 3.9, 3.10 and 3.14. It ran a 26-case probe of every union form in a fresh process per case. The output is byte-identical between base and head on 3.9, 3.10 and 3.14. On 3.8, union now raises dict's own TypeError.

@JarryShaw JarryShaw added the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 14, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 14, 2026
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 89.47% (unit tier, Python 3.15, 1691825be, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 3 20 3 95.16%
pcapkit/const 18860 954 2368 868 91.06%
pcapkit/corekit 2372 67 762 35 96.49%
pcapkit/dumpkit 275 3 100 3 98.40%
pcapkit/foundation 3035 151 1076 34 94.28%
pcapkit/interface 138 7 52 5 93.68%
pcapkit/protocols 17166 226 4580 190 98.07%
pcapkit/toolkit 1437 279 592 5 78.12%
pcapkit/utilities 494 4 146 4 98.75%
pcapkit/vendor 4417 2342 1010 157 43.38%

Per-file detail: the coverage-html artifact of this run.

@JarryShaw

Copy link
Copy Markdown
Owner Author

prefer to use version gates, instead of hasattr checks

@JarryShaw

Copy link
Copy Markdown
Owner Author

Recorded: a sys.version_info gate instead of hasattr(dict, '__or__'). A worker is on round 2; the verdict resets on the new head. The same applies to #1701's round 2, which uses sys.version_info blocks in compat.py in place of its getattr chain, and to #1709.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 14, 2026
…it (#1694)

`_Registries.__or__`/`__ror__` called `dict.__or__`/`dict.__ror__`
unconditionally. Dict union is PEP 584, Python 3.9 and up, so on 3.8 any
`|` or `|=` with the registry first imported TCP and UDP and then raised
`AttributeError`, where a plain dict raises `TypeError`.

- Define both only `if sys.version_info >= (3, 9)`. Before 3.9 the
  registry now raises dict's `TypeError`, naming its own type, and
  imports nothing; from 3.9 nothing changes. `|=` stays dict's in-place
  update, which reads no value. `__dir__` leaves out the new `sys`
  import, as it does `importlib`.
- Run the tests' `or`/`ror` views from 3.9 only, and pin which views ran.
- Add `test_union_is_dict_s_on_every_python`: all four `|`/`|=` forms
  against dict's own behaviour on the running interpreter.

Tests: the touched module passes on 3.8.20, 3.9.25, 3.10.21 and 3.14.8.
@JarryShaw
JarryShaw force-pushed the fix/1694-apptype-dict-union-py38 branch from 6be6781 to 1691825 Compare October 14, 2026 03:08
@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 14, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Verdict: GOOD TO GO at 1691825be. A Sonnet cross-review checked round 2. The hasattr(dict, '__or__') probe is now a sys.version_info >= (3, 9) gate in compat.py's tuple style. sys is hidden from the module's eager dir(), and branch coverage on 3.14 is 100% with the existing pragma. The reviewer ran its own 30 cases in a fresh process each, on 3.8, 3.9, 3.10 and 3.14. The 26 round-1 cases are byte-identical to round 1. On 3.9, 3.10 and 3.14 the results match main; on 3.8, every union form raises dict's own TypeError and imports nothing. The module passes on all four interpreters.

@JarryShaw
JarryShaw merged commit 7538258 into main Oct 14, 2026
51 checks passed
@JarryShaw
JarryShaw deleted the fix/1694-apptype-dict-union-py38 branch October 14, 2026 04:02
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 14, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 14, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Pull requests that fix a defect (fix: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

fix(const): the AppType registry's __or__ and __ror__ call dict.__or__, which Python 3.8 lacks

1 participant