Required/NonNull behaviour for Pydantic fields with defaults

[davidkell]

We think the following code produces an incorrect schema:

from pydantic import BaseModel
from graphene_pydantic import PydanticObjectType
import graphene

class ExampleModel(BaseModel):
	attr: int = 0

class ExampleType(PydanticObjectType):
	class Meta:
		model = ExampleModel

class Query(graphene.ObjectType):
    example = graphene.Field(ExampleType)

    @staticmethod
    def resolve_example(parent, info):
        # fetch actual PersonModels here
        return [ExampleType()]

schema = graphene.Schema(query=Query)

print(schema)

Result:

schema {
  query: Query
}

type ExampleType {
  attr: Int
}

type Query {
  example: ExampleType
}

AFAICT, the Int field on ExampleType should not be nullable, since there will always be a default value and this value cannot be null. Instead it should be:

type ExampleType {
  attr: Int!
}

I think it's a one line fix here, you need to change:

field_kwargs.setdefault("required", field.required)

to

field_kwargs.setdefault("required", not field.allow_none)

For reference, the Pydantic docs on distinguishing nullability (optional) and required fields.

If you agree with this, I'm happy to submit a PR - we are already doing this on our fork.

[necaris]

@daviddaskell thanks for this! I am not using graphene_pydantic any more at my work so not running into issues like this! I agree, not field.allow_none does seem to be the correct behavior -- I'd welcome the PR!

If you do end up submitting the PR, could you include the fix you suggested for Lists of non-null elements as well?

[davidkell]

Sure, happy to.

Am I correct in saying that the latest version only works with Graphene 3? In which case we'll probably have to keep a 0.1.0 fork going as well until the Graphene 3 release happens with all the dependent libraries.

[necaris]

0.1.0 fork going as well until the Graphene 3 release happens with all the dependent libraries.

That's right, currently 0.2.0 only supports Graphene 3. I'd welcome any changes that make it compatible with 2.x and 3.x, but also happy to maintain an 0.1.x branch (and release builds to PyPI).

[conao3]

@davidkell
I found this issue when using graphene-pydantic.
For some reason, the fields in list[str] don't work (#83), so only the changes you mention are needed first. Can you issue a PR?

[davidkell]

Here you go @conao3 - are you happy to merge in @necaris ?