django-ninja icon indicating copy to clipboard operation
django-ninja copied to clipboard

Documentation Issue - Optional field declarations incompatible with pydantic 2

Open sumebrius opened this issue 1 year ago • 3 comments

The documentation contains several examples of declaring optional fields within a Schema by simply providing a default value. However pydantic2 requires these fields to be explicitly typed as Optional

Instances I've found:

sumebrius avatar Dec 08 '23 00:12 sumebrius

Hi @sumebrius could you show on pydantic doc where they require this ?

Generally it works, but just not very type-accurate

vitalik avatar Dec 08 '23 07:12 vitalik

Hi @sumebrius could you show on pydantic doc where they require this ?

Generally it works, but just not very type-accurate

Not from the documentation, but the issue have been discussed in part here; https://github.com/pydantic/pydantic/issues/6546.

TobiasEdqvist avatar Dec 09 '23 00:12 TobiasEdqvist

Hmm... so checking through the pydantic docs, I can't seem to find much around optional/nullable fields - aside from this section in the migration guide to v2 warning that the logic has changed (and related to the issue @TobiasEdqvist linked.)

Doing some digging around in the docs, and a bit of experimentation myself - I think I have originally misunderstood the pydantic2 requirements/behaviour. It looks like:

  • field: str = None - If provided, must be a string (cannot be None/null). May not be provided in the payload, in which case field is set to None. There are actually several examples of this in the pydantic2 docs, but they mostly look like they were never updated from the old version docs)
  • field: Optional[str] - Must be provided, but may be None.
  • field: Optional[str] = None - May or may not be provided. May be None. Set to None if not provided.

The examples I referenced in the ninja docs aren't necessarily incorrect, but they do have implications that could lead new users to fall into pitfalls (like the one I did when upgrading to v1.0 :smiling_face_with_tear:) - I'd argue that one of the last 2 options would be "best practice" to encourage in the docs.

I'd be happy to submit a PR adding a small section on optional fields to the Schema docs, if you agree with the above.

sumebrius avatar Dec 09 '23 06:12 sumebrius