Skip to content

fix(protocols): decide auto-registration's abstractness without the interpreter's abstract flag (#2784) - #2785

Merged
JarryShaw merged 1 commit into
mainfrom
fix/2784-graalpy-abstract-check
Nov 5, 2026
Merged

fix(protocols): decide auto-registration's abstractness without the interpreter's abstract flag (#2784)#2785
JarryShaw merged 1 commit into
mainfrom
fix/2784-graalpy-abstract-check

Conversation

@JarryShaw

@JarryShaw JarryShaw commented Nov 5, 2026

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • fix — corrects a defect

Description of your pull request and other information

On GraalPy 25.4, an abstract Protocol subclass reads as concrete once it is made, so the experimental leg fails test_an_abstract_subclass_is_skipped (#2784).

The cause, measured on GraalPy 25.4.4: a class's __flags__ freezes the first time it is read, and setting __abstractmethods__ afterwards no longer sets the abstract bit. Protocol.__init_subclass__ calls inspect.isabstract, which reads __flags__ before abc.ABCMeta has set __abstractmethods__. That froze the flag on every subclass. Registration itself was right, so the issue's guess that abstract subclasses got registered was wrong.

The fix:

  • pcapkit.utilities.inspect.isabstract decides from the class namespace and the bases' __abstractmethods__, as ABCMeta does, without reading any flag. It gives the same answer during __init_subclass__ and after.
  • The hook uses it, so GraalPy's flag stays correct.
  • Tests: the registry tests check __abstractmethods__ instead of inspect.isabstract. New tests use a stand-in metaclass whose __flags__ freezes, or raises, when read, so they fail on unfixed main under CPython too.

Closes #2784

@JarryShaw
fix(protocols): decide auto-registration's abstractness without the i…
112bf86 
…nterpreter's abstract flag (#2784)

- GraalPy freezes a class's __flags__ at its first read. Protocol.__init_subclass__
  read it through inspect.isabstract before ABCMeta set __abstractmethods__, so every
  abstract Protocol subclass read as concrete afterwards; registration was right.
- Add pcapkit.utilities.inspect.isabstract, which derives the answer from the class
  namespace and the bases' __abstractmethods__ and reads no flag; the hook uses it.
- Tests: an __abstractmethods__ oracle, and a frozen-__flags__ stand-in for GraalPy.
@JarryShaw JarryShaw added fix Pull requests that fix a defect (fix: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Nov 5, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Nov 5, 2026
@github-actions github-actions Bot added this to the PyPCAPKit v1.6 milestone Nov 5, 2026
@JarryShaw JarryShaw mentioned this pull request Nov 5, 2026
Closed
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Coverage: 94.46% (unit tier, Python 3.15, 112bf8656, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 106 4 20 2 95.24%
pcapkit/const 20918 175 2908 156 98.57%
pcapkit/corekit 2918 77 942 38 96.81%
pcapkit/dumpkit 286 3 102 3 98.45%
pcapkit/foundation 3512 27 1218 32 98.75%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 22840 157 5950 127 99.00%
pcapkit/toolkit 961 26 342 8 96.62%
pcapkit/utilities 574 7 156 5 98.36%
pcapkit/vendor 5590 2286 1280 214 55.75%

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

@JarryShaw

JarryShaw commented Nov 5, 2026

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 112bf8656. A Sonnet cross-review probed 19 abstract-class shapes on CPython 3.14: properties, class and static methods, overrides, diamonds and non-ABCMeta mixins. In every case the new isabstract agreed with ABCMeta's __abstractmethods__, both during __init_subclass__ and after creation. ProtocolBase.__registry__ is byte-identical before and after the fix. The new hook test fails on unfixed main under CPython, and every changed file parses under 3.8. The docs build adds no warnings, and the required checks and every KTLO and experimental leg that has finished are green. GraalPy is still running, and I'll re-check its failing ids when it finishes.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Nov 5, 2026
@JarryShaw
JarryShaw merged commit f312f92 into main Nov 5, 2026
70 checks passed
@JarryShaw
JarryShaw deleted the fix/2784-graalpy-abstract-check branch November 5, 2026 02:10
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Nov 5, 2026
@JarryShaw

JarryShaw commented Nov 5, 2026

Copy link
Copy Markdown
Owner Author

GraalPy re-check, as promised: the leg on 112bf8656 finished with every shard passing and 0 failed. test_an_abstract_subclass_is_skipped, which fails on main before this PR, now passes on GraalPy.

@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Nov 5, 2026
JarryShaw added a commit that referenced this pull request Nov 5, 2026
@JarryShaw
docs(changelog): fold #2785 (#2449)
5a3ac19 
- Added: pcapkit.utilities.inspect and its isabstract, which decides a
  class's abstractness from its namespace and its bases' __abstractmethods__
  rather than the interpreter's abstract flag, and which Protocol's
  registration uses (#2785); the defect it fixes never shipped and is not
  described.
- Prepared ahead of #2785 merging, so main is not merged in yet.
- CHANGELOG.md regenerated; util/changelog_md.py --check passes.
@JarryShaw JarryShaw mentioned this pull request Nov 5, 2026
Merged
1 of 5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

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

Projects

PyPCAPKit
Status: Done

Milestone

PyPCAPKit v1.6

Development

Successfully merging this pull request may close these issues.

fix(protocols): auto-registration's abstract check misreads Protocol subclasses on GraalPy

1 participant

@JarryShaw