Skip to content

Derive macro for rich enums #208

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

evonreis
Copy link

@evonreis evonreis commented May 17, 2025

Add new attribute macro gen_stub_pyclass_rich_enum that creates internal classes for the variants of rich enums, that is, enums with variants that have fields.

Also creates in type_info.rs PyRichEnumInfo and VariantInfo for manual creation of stub information for rich enums.

Both of these are handled by 'ClassDef', which can now have internal classes and will write properly indented internal classes to the stub file.

This PR also fixes the implementation on tuples so that tuples of length one are supported.

@evonreis evonreis marked this pull request as draft May 17, 2025 20:08
This allows manual stub information to be entered in a manner that follows simple enums.

Add an example rich enum to the pure example.
@evonreis evonreis changed the title Derive classes for rich enums Derive macro for rich enums May 18, 2025
@evonreis evonreis marked this pull request as ready for review May 18, 2025 17:53
@jagregory
Copy link

This looks great @evonreis. I just tried it out and it nearly works as I'd expected with a couple of small issues if you're interested in feedback.

  1. The inner classes that PyO3 generates are subclassed from the outer class, but the typings in your PR don't reflect that.

    e.g. in your example it should be:

    class NumberRich:
        class FLOAT(NumberRich): # <-- inherits from NumberRich
            r"""
            Float variant
            """
            _0: builtins.float
    
        class INTEGER(NumberRich):
            r"""
            Integer variant
            """
            int: builtins.int
            r"""
            The integer value
            """

    This way if you have a function elsewhere you can define it as def something() -> NumberRich allowing either branch of the enum to be returned. Currently this isn't possible as FLOAT and INTEGER are not typed as subclasses.

  2. PyO3 generates default constructors for the classes which are missing from these typings, so currently it's not possible to construct these classes and make the type checker happy.

    class NumberRich:
        class FLOAT:
            # ... existing code ...
            __new__(cls, _0: builtins.float): ... # <-- default constructor
    
        class INTEGER:
            # ... existing code ...
            __new__(cls, int: builtins.int): ...

@jagregory
Copy link

I quickly hacked together a couple of commits which work. I'm not very familiar with the codebase so there might be better ways to achieve the same outcome. https://github.com/jagregory/pyo3-stub-gen/tree/rich_enum

  1. jagregory@53cc705 adds the base class, I just mimicked the way classbases works
  2. jagregory@33628da generates the default constructor. This one is hackier, I'm sure there's a better way to do it. I couldn't find an obvious way to return Self for __new__ so had to generate the return type manually. I didn't set import on the return type which I assume is ok because it's a self reference so it should be in scope already. For the args I didn't set the signature, I'm not sure what the consequence of (not) doing that might be for more complex types but it works ok for the example given.

@flying-sheep
Copy link
Contributor

@jagregory your proposed transform looks great!

pyo3 defines some runtime checks in the example code for complex enums, maybe we should transform these into an assert_type test suite?

Some more things that should be supported by the generated typing:

  1. __match_args__ so the type checker knows if users use the match statement right
  2. #[pyo3(constructor = (...))] support (if that’s not already supported)
  3. remaining methods of tuple variants:

@evonreis
Copy link
Author

Thank you @jagregory and @flying-sheep for looking this over. I will address all of the issues you've brought up.

This allows variants to be correctly returned from functions that
have a return type of the enum.

Avoids explicit setting of a base class in the macro by
creating an unqualified TypeInfo struct from the enum python class name
This avoids having to double-specify the enum python class name
when manually defining the stubs for a rich enum.
@evonreis
Copy link
Author

evonreis commented May 31, 2025

@jagregory, I've pushed a commit for the making the variant classes derived from the enum class in the stub file.

I build a TypeInfo for the enum on the fly when building ClassDef structs. This avoids having to specify the name of the enum in the VariantInfo struct, which in turn avoids a user having to enter the enum name for every variant if they are defining the stub by hand (not using the macro).

fc933e8

erikvonreis and others added 3 commits May 31, 2025 18:50
Shows derivation of variant classes from enum class.
PyO3 creates a __new__() method for constructing enum variants,
For tuple variants it creates __len__() and __getitem__() methods as well.

These now have stubs in the generated file.

__new__() was added in a previous commit.  This commit modifies the stub to match

[pyo3(constructor = ... )] attributes on variants that can change the signature of __new__().

Generation of the MethodInfo structs for these variant methods are moved out of generate/class.rs and
into a new file, variant_methods.rs
@evonreis
Copy link
Author

evonreis commented Jun 1, 2025

@jagregory I cherry-picked your commit for a variant __new__() method, and modified it to handle the #[pyo3(constructor = ...)] case.

__match_case__, __len__(), and __getitem__() are also added.

@flying-sheep I don't understand your testing suggestion, but certainly more tests are needed. I think your other points are now addressed.

@flying-sheep
Copy link
Contributor

flying-sheep commented Jun 2, 2025

I don't understand your testing suggestion, but certainly more tests are needed. I think your other points are now addressed.

This looks awesome, thank you!

Regarding testing, I meant that the example code I linked has assert isinstance(thing, MyEnum.Variant). I meant that if pyo3-stub-gen has some testing infrastructure like this, we should reproduce that example code as a test, with the change that assert isinstances should be replaced with assert_types.

But I think this project doesn’t have this kind of tests yet, so what you did is perfect!

@evonreis
Copy link
Author

evonreis commented Jun 3, 2025

@flying-sheep I understand your idea better now. I tried to write something, but assert_type() was introduced in py3.11 seemingly, and pyo3-stub-gen passes tests in py3.9. Getting assert_type() to work while still supporting 3.9 is beyond my python knowledge.

@flying-sheep
Copy link
Contributor

No worries, the exact tests you added before are perfect!

Not available in 3.9, the minimum python version and the default used for testing.
@jagregory
Copy link

What's remaining on this to be able to merge?

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.

4 participants