Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add module member name sorting to Stubgen #938

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

tlambert03
Copy link

hey @wjakob, thanks for the awesome package.

this relates to #899 (but may not be exactly the same thing). My stubs are getting generated out of order, and therefore have Undefined name errors for some default values when running things like ruff on the output, due to inspect.getmembers() sorting members alphabetically.

I recognize this is a tricky topic, so this PR is designed to do nothing by default, but leave a little bit of agency to the end user. It adds a new member_sort parameter to StubGen

class StubGen:
    def __init__(
        self,
        ...
        member_sort: Optional[Callable[[tuple[str, Any]], Any]] | Literal['as-defined'] = None,
    ) -> None:

if a Callable or None (the default) is passed, it is used directly in sorted(getmembers(value), key=self. member_sort)

if the literal string 'as-defined' is passed, then the module members are sorted as they are defined in the module (using the module __dict__).

let me know what you think

@tlambert03 tlambert03 changed the title feat: add member-sort feat: add module member name sorting to Stubgen Feb 17, 2025
Co-authored-by: riley-kinahan <[email protected]>
Co-authored-by: riley-kinahan <[email protected]>
@nurpax
Copy link
Contributor

nurpax commented Mar 11, 2025

Can this also be turned on the CLI or enabled in cmake rules?

I'd like to use this in the generated pyi file as this makes the output much more useful if it can retain the original module's logical API grouping. I imagine the pyi file being one of the primary ways the users of an extension can browse the API ref.

@tlambert03
Copy link
Author

I can look at adding that. But will wait to see what @wjakob thinks before going further

@nurpax
Copy link
Contributor

nurpax commented Mar 17, 2025

I wonder if the original problem statement is stated too verbosely for this to get traction. :) I still think this change is valuable.

The problem is that due to how Python APIs return module members, the symbols in the stubgen pyi output are by default sorted in alphabetical order. I would argue that this is the wrong default but that's how the Python API works. IMO a more useful default order would be the output exactly in the same order as the symbols are defined in the Nanobind extension code.

This change adds the option to keep the original order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants