feat(sqlalchemy-factory): Add support for SQLAlchemy custom types #398

bc291 · 2023-10-06T22:29:47Z

Pull Request Checklist

New code has 100% test coverage
(If applicable) The prose documentation has been updated to reflect the changes introduced by this PR
(If applicable) The reference documentation has been updated to reflect the changes introduced by this PR
Pre-Commit Checks were ran and passed
Tests were ran and passed

Description

Currently custom SQLAlchemy types are not supported which results in:

polyfactory/factories/base.py:715: in build
    return cast("T", cls.__model__(**cls.process_kwargs(**kwargs)))
polyfactory/factories/base.py:676: in process_kwargs
    for field_meta in cls.get_model_fields():
polyfactory/factories/sqlalchemy_factory.py:145: in get_model_fields
    fields_meta.extend(
polyfactory/factories/sqlalchemy_factory.py:147: in <genexpr>
    annotation=cls.get_type_from_column(column),
polyfactory/factories/sqlalchemy_factory.py:133: in get_type_from_column
    annotation = column.type.python_type
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = CustomType()

    @property
    def python_type(self) -> Type[Any]:
        """Return the Python type object expected to be returned
        by instances of this type, if known.
    
        Basically, for those types which enforce a return type,
        or are known across the board to do such for all common
        DBAPIs (like ``int`` for example), will return that type.
    
        If a return type is not defined, raises
        ``NotImplementedError``.
    
        Note that any type also accommodates NULL in SQL which
        means you can also get back ``None`` from any type
        in practice.
    
        """
>       raise NotImplementedError()
E       NotImplementedError

This PR adds support for custom types created by:

subclassing sqlalchemy.types.TypeDecorator; note: impl type needs to be python-mappable

class CustomType(types.TypeDecorator):
    impl = DateTime(timezone=True)

class Model(Base):
    __tablename__ = "example_table"

    id: orm.Mapped[int] = orm.mapped_column(primary_key=True)
    custom_type: orm.Mapped[Any] = orm.mapped_column(type_=CustomType(), nullable=False)

instance = ModelFactory.build()
assert isinstance(instance.custom_type, datetime.datetime)

subclassing sqlalchemy.types.UserDefinedType:

class CustomType(types.UserDefinedType):
    ...

class Model(Base):
    __tablename__ = "example_table"

    id: orm.Mapped[int] = orm.mapped_column(primary_key=True)
    custom_type: orm.Mapped[Any] = orm.mapped_column(type_=CustomType())

In this case, ParameterException is raised which encourages user to override get_sqlalchemy_types class method in the factory:

User defined type detected (subclass of {types.UserDefinedType}). Override get_sqlalchemy_types to provide factory function.

class ModelFactory(SQLAlchemyFactory[Model]):
    __model__ = Model

    @classmethod
    def get_sqlalchemy_types(cls) -> dict[Any, Callable[[], Any]]:
         return super().get_sqlalchemy_types() | {CustomType: lambda: cls.__faker__.date_time()}

...
instance = ModelFactory.build()
assert isinstance(instance.custom_type, datetime.datetime)

Close Issue(s)

…corator, UserDefinedType)

…` override tests

…ibility

github-actions · 2023-10-07T12:25:26Z

Documentation preview will be available shortly at https://litestar-org.github.io/polyfactory-docs-preview/398

guacs

This looks great! I just have a few comments/doubts :)

guacs · 2023-10-08T08:43:22Z

polyfactory/factories/sqlalchemy_factory.py

+        elif issubclass(column_type, types.UserDefinedType):
+            parameter_exc_msg = (
+                f"User defined type detected (subclass of {types.UserDefinedType}). "
+                "Override get_sqlalchemy_types to provide factory function."
+            )
+            raise ParameterException(parameter_exc_msg)


I don't think we need to throw the exception here itself. If the provider map doesn't have the corresponding factory function, then the exception will be thrown from the get_field_value method. Instead, this can just return the column type directly if the column type is a subclass off UserDefinedType.

guacs · 2023-10-08T08:53:20Z

polyfactory/factories/sqlalchemy_factory.py

@@ -118,6 +118,17 @@ def get_type_from_column(cls, column: Column) -> type:
            annotation = column_type
        elif issubclass(column_type, types.ARRAY):
            annotation = List[column.type.item_type.python_type]  # type: ignore[assignment,name-defined]
+        elif issubclass(column_type, types.TypeDecorator) and isinstance(
+            python_type := column_type.impl.python_type,


Why is the test for python_type being an instance of type needed? Also, what happens if there is no python_type implemented for the column_type.impl? For example, it could be set to postgresql.CIDR or types.ARRAY.

Essentially, I think the parsing of the annotation from the column type needs to be recursive in the case of types.TypeDecorator.

guacs · 2023-10-08T09:04:38Z

docs/usage/library_factories/sqlalchemy_factory.rst

+UserDefinedType based
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+``get_sqlalchemy_types`` classmethod needs to be overridden to provide factory function for custom type.


This can be changed so that the user should override the get_provider_map method. That makes it more consistent with how the other factories handle custom user defined types.

This provides the mapping from sqlalchemy types to provider where this is directly mapped from column vs where the implementation type could be used.

I think these could be unified but may require some reworking so the the logic of handling types in exposed separately. There wasn't a clear way to map things like Integer column without repeating int handling. This case is alright to handle but this becomes more complex especially when considering things like list[str].

This provides the mapping from sqlalchemy types to provider where this is directly mapped from column vs where the implementation type could be used.

I think these could be unified but may require some reworking so the the logic of handling types in exposed separately. There wasn't a clear way to map things like Integer column without repeating int handling. This case is alright to handle but this becomes more complex especially when considering things like list[str].

I agree that we can keep a separate get_sqlalchemy_types and a get_provider_map so that we can deal with sql types in an easier manner. However, since this is a user defined type, should it not go into the get_provider_map which is where the user defined types are supposed to be given for the other factories?

Not with the current implementation.

get_type_from_column only considers the output of get_sqlalchemy_types here for SQLA types it should not map for providers. This could be changed to just using get_provider_map to bring inline with other factories.

I don't think this is a backwards breaking change though one very minor concern it may be harder where want to provide a mapping for a field with type of Column itself. This is minor as feels a fairly obscure use case.

guacs · 2023-10-08T09:08:48Z

@adhtruong If you have time, could you take a look at this as well?

adhtruong · 2023-10-08T10:25:04Z

tests/sqlalchemy_factory/test_sqlalchemy_factory_v2.py

@@ -85,3 +87,95 @@ class ModelFactory(SQLAlchemyFactory[Model]):

    instance = ModelFactory.build()
    assert instance.overridden is not None
+
+
+@pytest.mark.parametrize(


Is this applicable to SQLA 1.4? If so, I think these tests should be moved to _common and adjusted to common so tested against both 1.4 and 2

guacs · 2023-10-14T11:40:33Z

@bc291 any updates?

adhtruong · 2024-04-28T12:04:08Z

Thanks for the work on this @bc291 . I think this use case is now covered by https://github.com/litestar-org/polyfactory/pull/513/files. Please reopen if missed off a use case here. You will need to sync with main

bc291 added 8 commits October 6, 2023 23:57

feat(sqlalchemy-factory): support for SQLAlchemy custom types (TypeDe…

cef52d6

…corator, UserDefinedType)

feat(sqlalchemy-factory): add UserDefinedType/`get_sqlalchemy_types…

7c430ac

…` override tests

fix(sqlalchemy-factory): swap dict -> Dict for py3.8 compatibility

ed1d3b8

fix(sqlalchemy-factory): import Callable from typing for py3.8 compat…

0c0f05c

…ibility

fix(sqlalchemy-factory): merge dicts with ** for py3.8 compatibility

c6bf43c

fix(sqlalchemy-factory): revert pdm files

16d0c93

feat(sqlalchemy-factory): add SQLAlchemy custom types section to docs

19918fa

fix(sqlalchemy-factory): alter UserDefinedType example caption

9348488

bc291 marked this pull request as ready for review October 7, 2023 12:25

bc291 requested review from a team as code owners October 7, 2023 12:25

guacs requested changes Oct 8, 2023

View reviewed changes

adhtruong requested changes Oct 8, 2023

View reviewed changes

adhtruong closed this Apr 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(sqlalchemy-factory): Add support for SQLAlchemy custom types #398

feat(sqlalchemy-factory): Add support for SQLAlchemy custom types #398

bc291 commented Oct 6, 2023 •

edited

Loading

github-actions bot commented Oct 7, 2023

guacs left a comment

guacs Oct 8, 2023

guacs Oct 8, 2023

guacs Oct 8, 2023

guacs Oct 8, 2023

adhtruong Oct 8, 2023

guacs Oct 8, 2023

adhtruong Oct 8, 2023

guacs commented Oct 8, 2023

adhtruong Oct 8, 2023 •

edited

Loading

guacs commented Oct 14, 2023

adhtruong commented Apr 28, 2024

feat(sqlalchemy-factory): Add support for SQLAlchemy custom types #398

feat(sqlalchemy-factory): Add support for SQLAlchemy custom types #398

Conversation

bc291 commented Oct 6, 2023 • edited Loading

Pull Request Checklist

Description

Close Issue(s)

github-actions bot commented Oct 7, 2023

guacs left a comment

Choose a reason for hiding this comment

guacs Oct 8, 2023

Choose a reason for hiding this comment

guacs Oct 8, 2023

Choose a reason for hiding this comment

guacs Oct 8, 2023

Choose a reason for hiding this comment

guacs Oct 8, 2023

Choose a reason for hiding this comment

adhtruong Oct 8, 2023

Choose a reason for hiding this comment

guacs Oct 8, 2023

Choose a reason for hiding this comment

adhtruong Oct 8, 2023

Choose a reason for hiding this comment

guacs commented Oct 8, 2023

adhtruong Oct 8, 2023 • edited Loading

Choose a reason for hiding this comment

guacs commented Oct 14, 2023

adhtruong commented Apr 28, 2024

bc291 commented Oct 6, 2023 •

edited

Loading

adhtruong Oct 8, 2023 •

edited

Loading