graphql-python / graphene-pydantic

Integrate GraphQL with your Pydantic models
Other
229 stars 44 forks source link

Pydantic -> Graphene type conversion breaks when using freezegun #56

Open emasatsugu opened 3 years ago

emasatsugu commented 3 years ago

this schema

class TestSchema(BaseModel):
    date_field: datetime.date

breaks in tests that use freezegun to freeze time:

E   graphene_pydantic.converters.ConversionError: Don't know how to convert the Pydantic field ModelField(name='date_field', type=date, required=True) (<class 'datetime.date'>)

I believe the issue is because freezegun overwrites the type of datetime.date and datetime.datetime, so these lines in the graphene_pydantic converter (find_graphene_type()) don't evaluate to true:

elif type_ == datetime.date:
    return Date

pydantic code: https://github.com/graphql-python/graphene-pydantic/blob/master/graphene_pydantic/converters.py#L186 freezegun code: https://github.com/spulec/freezegun/blob/master/freezegun/api.py#L637 related freezegun issue: https://github.com/spulec/freezegun/issues/170

I'm not sure if this is a weird fix or not, but changing the if condition to:

elif type_.__name__ == "date"

or

elif issubclass(type_, datetime.date):

fixes this use case.

A better suggestion (though I don't know the best way to implement) is to allow a custom type mappings so we don't have to rely on this switch statement.

nick-merrill commented 1 year ago

~I would be happy if we change all instances of type_ == SOME_TYPE to issubclass(type_, SOME_TYPE) (@emasatsugu's second suggestion).~

~That seems like a desirable change, since someone might also want to subclass the datetime class and have that be supported by this library.~

Edit: Actually, I see that type_ is not always a class, so we can't quite do what I was hoping.

necaris commented 1 year ago

Edit: Actually, I see that type_ is not always a class, so we can't quite do what I was hoping.

Unfortunately it's a bit trickier, yes. We can add specific call-outs to subclasses as well, which might solve the issue.