agronholm / agronholm/sqlacodegen
Using types instead of strings in return values of methods in generators
- Langage dominant
- Python
- Étoiles
- 2.4k
- Forks
- 284
- Merge moyen
- 2 j 1 h
- PR mergées (30 j)
- 1
Description
### Things to check first
- [X] 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
```python
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
```python
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
```python
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
```python
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.
Guide de contribution
Ouvrir le guide de contribution
Évaluation
Cette issue n'a pas encore été évaluée.