Skip to content

Resolve Inheritance Sort Issue#238

Merged
ax3l merged 13 commits intopybind:mainfrom
ax3l:test-inheritance-sort-issue
Apr 15, 2026
Merged

Resolve Inheritance Sort Issue#238
ax3l merged 13 commits intopybind:mainfrom
ax3l:test-inheritance-sort-issue

Conversation

@ax3l
Copy link
Copy Markdown
Contributor

@ax3l ax3l commented Jan 9, 2025 •
edited
Loading

This modifies the inheritance unit test to demonstrate the sorting issue in #231.

Classes in generated .pyi stubs were sorted alphabetically, causing derived classes to appear before their base classes and breaking type checkers. Likewise, class-body dependencies were not sorted properly, creating invalid Python stubs.

Three changes fix this:

  • Parser: use module.__dict__.items() instead of inspect.getmembers() to preserve the pybind11 registration order (definition order)
  • Printer: replace alphabetical sort with a configurable _order_classes() dispatch supporting "definition" (default), "topological" (Kahn's algorithm ensuring bases precede dependent (derived classes, class-body type references)), and "alphabetical"
  • CLI: add --sort-by option to select the class ordering strategy

The topological sort ignores external bases (from other modules) and breaks ties by input position for deterministic output. Cyclic cross-references between classes (e.g. aliases, method signatures) are no inheritance cycles and are already handled by from __future__ import annotations in the generated stubs.

Fixes #231
Closes #294
Closes #275

Based on the approaches in PR #275 by @juelg and PR #294 by @daltairwalter, informed by review feedback from @skarndev and @sizmailov. Combined via Claude Code and reviewed via Codex, and manually.

@ax3l ax3l mentioned this pull request Jan 9, 2025
Closed
@sizmailov sizmailov force-pushed the test-inheritance-sort-issue branch from 2552158 to e083ebc Compare February 5, 2025 08:56
@juelg juelg mentioned this pull request Nov 7, 2025
Closed
2 tasks
@ax3l
Copy link
Copy Markdown
Contributor Author

ax3l commented Jan 22, 2026

Continued in #275

@ax3l ax3l closed this Jan 22, 2026
@ax3l ax3l added the bug Something isn't working label Jan 22, 2026
@ax3l ax3l reopened this Apr 2, 2026
@ax3l ax3l force-pushed the test-inheritance-sort-issue branch from e083ebc to f1ae578 Compare April 2, 2026 23:46
@ax3l ax3l changed the title Test: Inheritance Sort Issue Resolve Inheritance Sort Issue Apr 3, 2026
@ax3l ax3l force-pushed the test-inheritance-sort-issue branch from d93d341 to 5a184c7 Compare April 3, 2026 04:48
@ax3l
Test: Inheritance Sort Issue
19d3953 
This modifies the inheritance unit test to demonstrate
the sorting issue.
@ax3l ax3l force-pushed the test-inheritance-sort-issue branch 2 times, most recently from 1e96b24 to 07662e4 Compare April 3, 2026 05:24
ax3l and others added 2 commits April 2, 2026 22:37
@ax3l
More Tests: Cyclic
0b039d0
@ax3l @navinpv
@daltairwalter @skarndev @sizmailov @claude
Fix class ordering to preserve inheritance hierarchy (pybind#231)
13dbbf3 
Classes in generated .pyi stubs were sorted alphabetically, causing
derived classes to appear before their base classes and breaking type
checkers.

Three changes fix this:

- Parser: use module.__dict__.items() instead of inspect.getmembers()
  to preserve the pybind11 registration order (definition order)
- Printer: replace alphabetical sort with a configurable _order_classes()
  dispatch supporting "definition" (default), "topological" (Kahn's
  algorithm ensuring bases precede derived classes), and "alphabetical"
- CLI: add --sort-by option to select the class ordering strategy

The topological sort ignores external bases (from other modules) and
breaks ties by input position for deterministic output. Cyclic cross-
references between classes (e.g. aliases, method signatures) are not
inheritance cycles and are already handled by `from __future__ import
annotations` in the generated stubs.

Closes pybind#231

Based on the approaches in PR pybind#275 by @juelg and PR pybind#294 by
@daltairwalter, informed by review feedback from @skarndev and
@sizmailov.

Co-Authored-By: juelg <20750040+juelg@users.noreply.github.com>
Co-Authored-By: daltairwalter <daltairwalter@users.noreply.github.com>
Co-Authored-By: skarndev <skarndev@users.noreply.github.com>
Co-Authored-By: sizmailov <sizmailov@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ax3l ax3l force-pushed the test-inheritance-sort-issue branch from 07662e4 to 13dbbf3 Compare April 3, 2026 05:37
@ax3l ax3l mentioned this pull request Apr 3, 2026
Closed
@ax3l ax3l requested review from skarndev and virtuald April 3, 2026 06:09
This was referenced Apr 3, 2026
Open
Open
@ax3l
Update README.md
8ffd952 
New `--sort-by` CLI option
skarndev
skarndev reviewed Apr 6, 2026
View reviewed changes
Comment thread pybind11_stubgen/printer.py Outdated
@ax3l @skarndev
Improve sort_by typehint
a4e96f5 
Co-authored-by: Skarn <skarnproject@gmail.com>
ax3l added 2 commits April 7, 2026 21:32
@ax3l
Fix class ordering for runtime sibling references in stubs
589bd68 
Extend topological class ordering in the printer to account for
executable class-body dependencies, not just inheritance. In addition
to base-class edges, the sorter now treats class aliases and
print-safe field values that reference sibling classes as runtime
dependencies, deduplicates edges, and updates cycle diagnostics to
reflect the broader graph. This keeps generated .pyi files valid
Python without moving declarations out of class bodies, preserving
the conventional stub shape that type checkers expect.

Before:
```py
class ParIterBase:
  level: int

class ParticleContainer:
  name: str
  Iterator = ParIter
  def process(self, arg0: ParIter) -> None: ...

class ParIter(ParIterBase):
  def __init__(self, particle_container: ParticleContainer, level: int) -> None: ...
```

After:
```py
class ParIterBase:
  level: int

class ParIter(ParIterBase):
  def __init__(self, particle_container: ParticleContainer, level: int) -> None: ...

class ParticleContainer:
  name: str
  Iterator = ParIter
  def process(self, arg0: ParIter) -> None: ...
```
@ax3l
Update Silver Stubs
17d6aa4
@ax3l
Copy link
Copy Markdown
Contributor Author

ax3l commented Apr 8, 2026

I do not think we can easily support robust --sort-by definition if we have class-body dependencies, in addition to base-class edges, not just inheritance. I would remove the option.

@ax3l
Remove --sort-by definition
51dfa72 
Remove the CLI option, we cannot robustly support it with class-body
dependencies.
@ax3l ax3l force-pushed the test-inheritance-sort-issue branch from d363ee4 to 51dfa72 Compare April 8, 2026 04:49
ax3l added 2 commits April 7, 2026 21:52
@ax3l
Test: Fix Misleading Comment (Order)
3f129ac
@ax3l
Fix: Module Traversal & External Names
49e181e 
In `pybind11_stubgen/parser/mixins/parse.py`, module traversal now
preserves `module.__dict__` definition order for normal pybind11
exports but also appends lazily exposed members from
`dir()`/`getattr()`, so the PR keeps the ordering fix without
regressing PEP 562-style module attributes.

In `pybind11_stubgen/printer.py`, class dependency edges now use the
first identifier component of a reference, not the last. That fixes
the false-local-match issue for external names like `pkg.other.Foo`,
and it also makes dotted local references like `Outer.Inner` depend
on `Outer`, which is the actual runtime lookup requirement.
@ax3l ax3l requested review from sizmailov and skarndev April 8, 2026 05:15
@ax3l
Preserve class-local member definition order
acca73d 
`handle_class()` now uses a new _iter_class_members() helper that:

- preserves class_.__dict__ definition order for class-local members
- then appends additional visible members from dir()/getattr() so
  inherited or lazily exposed names are still seen

That makes nested class discovery consistent with the earlier
module-level fix, so the printer's topological sort now gets real
definition order as its stable tie-break for nested classes too.
@ax3l ax3l force-pushed the test-inheritance-sort-issue branch from 185a5b8 to acca73d Compare April 8, 2026 05:39
virtuald
virtuald approved these changes Apr 15, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@virtuald virtuald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly wary of having a home grown topological sort implementation here without a thorough test of the algorithm directly (would probably just vendor toposort if I was making this PR). But, it all looks right, so I guess that's fine.

@ax3l
Copy link
Copy Markdown
Contributor Author

ax3l commented Apr 15, 2026

Good point, we could depend on the toposort package or vendor it. But on the other side, Kahn's algorithm is textbook, so maybe rolling it is fine for now. We also have some details, like breaking ambiguities by asking the definition order, so maybe this will be quite robust going forward if we add more and more tests for potential defects.

@ax3l ax3l merged commit e0aa238 into pybind:main Apr 15, 2026
20 checks passed
@ax3l ax3l deleted the test-inheritance-sort-issue branch April 15, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@virtuald virtuald virtuald approved these changes

@sizmailov sizmailov Awaiting requested review from sizmailov

@skarndev skarndev Awaiting requested review from skarndev

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Sorting is causing parent-child relationships to be defined out of order

3 participants

@ax3l @virtuald @skarndev