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

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

Open
emasatsugu opened this issue Mar 2, 2021 · 2 comments
Open

Comments

@emasatsugu
Copy link

emasatsugu commented Mar 2, 2021

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: spulec/freezegun#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
Copy link

nick-merrill commented May 17, 2023

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
Copy link
Collaborator

necaris commented May 17, 2023

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.

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

No branches or pull requests

3 participants