datamodel-code-generator icon indicating copy to clipboard operation
datamodel-code-generator copied to clipboard

Differentiate "required with default", "required nullable", and "optional" properties

Open amh4r opened this issue 4 years ago • 12 comments
trafficstars

:warning: I'm willing to work on this change myself. But before starting, I'd like to discuss it.

Is your feature request related to a problem? Please describe.

Here's a repro: https://github.com/goodoldneon/datamodel-code-generator-Repro

OpenAPI allows you to specify properties as:

  • Required
  • Required with default
  • Required nullable
  • Unrequired

However, mashalling the pydantic models (e.g. BaseModel.dict()) will not differentiate them.

For example, let's say we have the following OpenAPI schema:

Foo:
  type: object
  properties:
    required:
      type: integer
    required_default:
      type: integer
      default: 2
    required_nullable:
      type: integer
      nullable: true
    unrequired:
      type: integer
  required:
    - required
    - required_default
    - required_nullable

If I marshall the generated model into JSON, I expect that all of the required* properties will always exist but the unrequired property may not.

If I instantiate Foo like this:

Foo(required=1, required_nullable=None)

Then I'd expect the marshalled JSON to be:

{
  "required": 1,
  "required_default": 2,
  "required_nullable": null,
}

But that doesn't seem to be possible.

Describe the solution you'd like

I thought I could do this with a new BaseModel.dict() argument, but the maintainers of pydantic rejected my PR:

https://github.com/samuelcolvin/pydantic/discussions/2837

They provided a (seemingly) good solution to the problem on their end. This makes me think the solution for datamodel-code-generator would be a new CLI arg that creates models like @PrettyWood's example.

Describe alternatives you've considered

I considered making a change to pydantic but my PR was rejected.

amh4r avatar Jun 05 '21 01:06 amh4r

@goodoldneon Thank you for creating this issue. I see.

They provided a (seemingly) good solution to the problem on their end. This makes me think the solution for datamodel-code-generator would be a new CLI arg that creates models like @PrettyWood's example.

How do you want to realize this CLI arg? I think you can use --base-class BASE_CLASS to change BaseModel. Or, did you need another way?

koxudaxi avatar Jun 05 '21 14:06 koxudaxi

I could use --base-class for the base class that overrides dict. But is there an existing way to add the unset default for unrequired properties? Unrequired fields need to default to a token/sentinel instead of None

amh4r avatar Jun 06 '21 14:06 amh4r

I guess you can use prettyWood's custom base class with --base-class.

your_app/custom_base.py

from enum import Enum, auto
from typing import Any, Dict, Optional, Union

from pydantic import BaseModel as PydanticBaseModel, Field


class Unrequired(Enum):
    token = auto()


unset = Unrequired.token


class CustomBaseModel(PydanticBaseModel):
    def dict(self, *, exclude_unset: bool = False, **kwargs: Any) -> Dict[str, Any]:
        d = super().dict(**kwargs)
        if exclude_unset:
            return {k: v for k, v in d.items() if v is not unset}
        return d

generate models.

$ datamodel-codegen --input openapi.yaml --base-class custom_base.CustomBaseModel --output your_app/models.py

your_app/models.py

# generated by datamodel-codegen:
#   filename:  openapi.yaml
#   timestamp: 2021-06-06T15:34:34+00:00

from __future__ import annotations

from typing import Optional

from custom_base import CustomBaseModel


class Foo(CustomBaseModel):
    required: int
    required_default: int
    required_nullable: int
    unrequired: Optional[int] = None

Is this way what you want?

koxudaxi avatar Jun 06 '21 15:06 koxudaxi

But is there an existing way to add the unset default for unrequired properties? Unrequired fields need to default to a token/sentinel instead of None

I'm sorry, I have not answered it yet. the custom template may be able to set the default. But, we may need a flag to detect it.

koxudaxi avatar Jun 06 '21 17:06 koxudaxi

I think your example would work if instead of this:

unrequired: Optional[int] = None

It was this:

unrequired: Union[int, Unrequired] = unset

amh4r avatar Jun 06 '21 17:06 amh4r

One more advantage to this unset approach is that Python code can differentiate between "property doesn't exist" and "property is null" in request bodies. Currently, our Python code has to treat both of them as None. I could check request_body.whatever is unset or request_body.whatever is None

amh4r avatar Jun 06 '21 18:06 amh4r

I feel that great approach. We should change the below code. https://github.com/koxudaxi/datamodel-code-generator/blob/dd49187c1853367dc704cc219c48a67fb4487535/datamodel_code_generator/model/base.py#L57-L69 https://github.com/koxudaxi/datamodel-code-generator/blob/dd49187c1853367dc704cc219c48a67fb4487535/datamodel_code_generator/model/pydantic/base_model.py#L77-L100

koxudaxi avatar Jun 06 '21 18:06 koxudaxi

Sounds good! I'll work on it this week

amh4r avatar Jun 06 '21 18:06 amh4r

did this solution reach any published version?

I have the following situation:

  properties:
...
     XTpUlt_ConsCCP_5a:
       type: array
          items:
            type: number
            format: float
            nullable: true
...

that generates something like this:

    XTpUlt_ConsCCP_5a: List[float]

but I need it to create something like this:

    XTpUlt_ConsCCP_5a: List[Optional[float]]

As this accepts nan values inside the list.

Something I noticed is that the nullable: true changes nothing

ricoms avatar Jun 29 '21 04:06 ricoms

@ricoms Thank you for this feedback. We have not resovled the problem yet.

koxudaxi avatar Jul 01 '21 15:07 koxudaxi

@ricoms I believe your problem is separate from this issue. You can get List[Optional[float]] using oneOf:

properties:
  XTpUlt_ConsCCP_5a:
    type: array
    items:
      oneOf:
        - type: 'null'
        - type: number
          format: float

amh4r avatar Jul 06 '21 02:07 amh4r

@koxudaxi sorry I've slept on this for so long. I decided to give it another shot in #670. There's still some work left, but I was hoping to get some feedback from you on what I have so far. Thanks for taking a look when you get the chance!

amh4r avatar Jan 03 '22 23:01 amh4r