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

proper support of __get_pydantic_core_schema__ #646

Open
jbkoh opened this issue Nov 20, 2024 · 7 comments
Open

proper support of __get_pydantic_core_schema__ #646

jbkoh opened this issue Nov 20, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@jbkoh
Copy link

jbkoh commented Nov 20, 2024

Summary

I'd like to dynamically add non-public methods to my Enums

What is the feature request for?

The core library

The Problem

I'm mixing up betterproto.Enum with pydantic classes for easier conversion between my database objects and protobuf messages. To use pydantic, I have to add validator based on this instruction. Thus, I decided to dynamically add a class method to a generated betterproto.Enum. However, betterproto.enum does not allow it by this

Are there any other parts of the code affected by adding a custom function __get_pydantic_core_schema__ to my generated Enum?

The Ideal Solution

I'd like __set_attr__ to accept new attributes if the new attributes are private (i.e.,starting with __) I can contribute if this direction is acceptable.

The Current Solution

If no other solutions, I will fork this repo and just remove the constraint. I don't like to fork for a hack.

@jbkoh jbkoh added the enhancement New feature or request label Nov 20, 2024
@Gobot1234
Copy link
Collaborator

Gobot1234 commented Nov 20, 2024

Would it be more useful to just add support in the root Enum for this? Can you not serialise an Enum without a custom implementation even though it's just a custom int?

@jbkoh
Copy link
Author

jbkoh commented Nov 20, 2024

I actually misread the original guideline from Pydantic and just found that I don't need to modify betterproto, even though if we can add such a handler to the codegen step to betterproto.Enum, I can avoid some headaches. Can we add a custom handler in the generation step as a configuration? I guess there are many other cases to "augment" the generated classes with some helper functions.

In other words, yes, it'd be useful to add this to the root Enum. I just don't know if betterproto users would like a dependency to pydantic. I was dumb that betterproto already has a dependency to it.

I didn't fully get the other question.

Can you not serialise an Enum without a custom implementation even though it's just a custom int?

@Gobot1234
Copy link
Collaborator

For the other question I was mainly just surprised that pydantic can't do this already

@jbkoh jbkoh changed the title allow setattr for additional methods of Enum proper support of __get_pydantic_core_schema__ Nov 20, 2024
@jbkoh
Copy link
Author

jbkoh commented Nov 20, 2024

While I wass trying to add this to Enum class, I found that --custom_opt=pydantic_dataclasses already exists and there __get_pydantic_core_schema__ is inserted as found in tests/output_betterproto_pydantic/enum/__init__.py. However, when I activated that option in my codegen, that pydantic function wasn't added. Any idea what I should look at?

Following is the command I used:

          protoc \
            --plugin=protoc-gen-custom=${betterproto_loc} \
            --custom_opt=pydantic_dataclasses \
            --python_out=packages/python \
            --experimental_allow_proto3_optional \
            --python_betterproto_out=packages/python \
            --proto_path=./ $(find myproject -iname "*.proto")

@jbkoh
Copy link
Author

jbkoh commented Nov 21, 2024

Found that I was using a wrong option. Somehow test code uses --custom_opt and the README says --python_betterproto_opt. It's all good now. Closing.

@jbkoh jbkoh closed this as completed Nov 21, 2024
@jbkoh jbkoh reopened this Dec 9, 2024
@jbkoh
Copy link
Author

jbkoh commented Dec 9, 2024

@Gobot1234

I actually need more help than what it has right now because it currently just treats betterproto.Enum as int at the Pydantic side. The currently generated code snippet is

    @classmethod
    def __get_pydantic_core_schema__(cls, _source_type, _handler):
        from pydantic_core import core_schema
        return core_schema.int_schema(ge=0)

and binds an enum field as an int. However, I want to preserve the custom enum field. I actually needed one in the original link, something like below.

        from pydantic_core import core_schema
        def validate(value: int) -> "MyCustomEnum":
            return cls(value)

        # Return the schema for validation and serialization
        return core_schema.chain_schema(
            [
                core_schema.int_schema(ge=0),  # Validate as a string first
                core_schema.no_info_plain_validator_function(validate),  # Custom validation
            ]
        )

It seems to be desired in general, and I'm curious if we can update the generation snippet to this.

@jbkoh
Copy link
Author

jbkoh commented Dec 9, 2024

This PR works for me, though I can add more tests to it:
#651

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

No branches or pull requests

2 participants