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

Using types instead of strings in return values of methods in generators #328

Open
1 task done
nixypanda opened this issue May 31, 2024 · 0 comments
Open
1 task done

Comments

@nixypanda
Copy link

Things to check first

  • I have searched the existing issues and didn't find my feature already requested there

Feature description

Usecase

Due to historical reasons I have to deal with a codebase where we are converting Uuid's to string when we fetch them from database, as a result we need to look at the custom generator logic in particular the following functions

  • render_column_type
  • render_column_attribute

The current custom generator

Right now both these functions return str which is hard to make changes too. I was wondering if you would be open to modifying these interfaces to return some types here, so these types are easy to modify.

I will take the example of modifying these for the aforementioned use case. To satisfy the condition I have done the following

    def render_column_type(self, coltype: object) -> str:
        from_lib = super().render_column_type(coltype)
        if from_lib == "Uuid" and isinstance(coltype, Uuid):
            return render_callable(coltype.__class__.__name__, kwargs={"as_uuid": False})
        return from_lib

    def render_column_attribute(self, column_attr: ColumnAttribute) -> str:
        column_python_type = "str"
        column = column_attr.column
        rendered_column = self.render_column(column, column_attr.name != column.name)

        if column.nullable:
            self.add_literal_import("typing", "Optional")
            column_python_type = f"Optional[{column_python_type}]"

        if isinstance(column_attr.column.type, Uuid):
            return f"{column_attr.name}: Mapped[{column_python_type}] = {rendered_column}"

        return super().render_column_attribute(column_attr)

As you can see this is a lot of code for something extremely trivial.

Proposed feature (or rather modification)

Let's assume we had types like the following

class RenderedColumnType:
    coltype: object
    args: list[str]
    kwargs: dict[str, str]

class RenderedColumnAttribute:
   column_attr: ColumnAttribute
   column_python_base_type: str
   nullable: bool
   rendered_column: RenderedColumn

And the methods on the generators had the following interface

def render_column_type(self, coltype: object) -> RenderedColumnType:
def render_column_attribute(self, column_attr: ColumnAttribute) -> RenderdColumnAttribute:

The the code that a consumer of this API would have to write would have been something like

The Custom Generator with these proposed changes

    def render_column_type(self, coltype: object) -> RenderedColumnType:
        from_lib = super().render_column_type(coltype)
        if isinstance(coltype, Uuid):
            from_lib.kwargs["as_uuid"] = False
        return from_lib

    def render_column_attribute(self, column_attr: ColumnAttribute) -> RenderedColumnAttribute:
        from_lib = super().render_column_attribute(column_attr)
        if isinstance(column_attr.column.type, Uuid):
            from_lib.column_python_base_type = "str"
        return from_lib

Which, imo, offers a far simpler interface for the consumer of this API to work with.

I have to admit that I have only thought about this from the use case that I encountered, and have not considered the overall impact on the generators. For that I can definitely use your insights, and lastly if this is something you would be interested in pursuing let me know, I can try to cook a PR to test out how this will look like.

Last but certainly not least, thanks for this awesome project it has been super useful.

Use case

Due to historical reasons I have to deal with a codebase where we are converting Uuid's to string when we fetch them from database, as a result we need to look at the custom generator logic in particular the following functions

  • render_column_type
  • render_column_attribute

Most of the details about this are already covered in the feature description section.

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

No branches or pull requests

1 participant