graphene-pydantic icon indicating copy to clipboard operation
graphene-pydantic copied to clipboard

Pydantic -> Graphene type conversion breaks when using freezegun

Open emasatsugu opened this issue 4 years ago • 2 comments

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.

emasatsugu avatar Mar 02 '21 05:03 emasatsugu

~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.

nick-merrill avatar May 17 '23 14:05 nick-merrill

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.

necaris avatar May 17 '23 15:05 necaris