Open
Conversation
sizmailov
force-pushed
the
test-inheritance-sort-issue
branch
from
2552158 to
e083ebc
Compare
2 tasks
Contributor
Author
|
Continued in #275 |
ax3l
force-pushed
the
test-inheritance-sort-issue
branch
from
e083ebc to
f1ae578
Compare
ax3l
force-pushed
the
test-inheritance-sort-issue
branch
from
d93d341 to
5a184c7
Compare
This modifies the inheritance unit test to demonstrate the sorting issue.
ax3l
force-pushed
the
test-inheritance-sort-issue
branch
2 times, most recently
from
1e96b24 to
07662e4
Compare
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 <[email protected]> Co-Authored-By: daltairwalter <[email protected]> Co-Authored-By: skarndev <[email protected]> Co-Authored-By: sizmailov <[email protected]> Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
ax3l
force-pushed
the
test-inheritance-sort-issue
branch
from
07662e4 to
13dbbf3
Compare
This was referenced Apr 3, 2026
Open
New `--sort-by` CLI option
skarndev
reviewed
Apr 6, 2026
|
|
||
| class Printer: | ||
| def __init__(self, invalid_expr_as_ellipses: bool): | ||
| def __init__(self, invalid_expr_as_ellipses: bool, sort_by: str = "definition"): |
Contributor
There was a problem hiding this comment.
I suggest an explicit typehint to be more clear on the options.
Suggested change
| def __init__(self, invalid_expr_as_ellipses: bool, sort_by: str = "definition"): | |
| def __init__(self, invalid_expr_as_ellipses: bool, sort_by: Literal["definition", "topological"] = "definition"): |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This modifies the inheritance unit test to demonstrate the sorting issue in #231.
Classes in generated
.pyistubs were sorted alphabetically, causing derived classes to appear before their base classes and breaking type checkers.Three changes fix this:
module.__dict__.items()instead ofinspect.getmembers()to preserve the pybind11 registration order (definition order)_order_classes()dispatch supporting "definition" (default), "topological" (Kahn's algorithm ensuring bases precede derived classes), and "alphabetical"--sort-byoption to select the class ordering strategyThe 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 annotationsin 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.