Pydantic: dict(skip_defaults=True) doesn't apply to nested models

[skewty]

from typing import List
from pydantic import BaseModel, Schema

DIGITAL_INPUT = 'digital_input'


class Payload(BaseModel):
    """Base class for all Combox payloads"""


class PointData(Payload):
    name: str = Schema(..., title='Name', min_length=1, max_length=32)
    gios_type: str = Schema(DIGITAL_INPUT, title='GIOS Type')


class PanelData(Payload):
    name: str = Schema(..., title='Name', min_length=1, max_length=32)
    points: List[PointData] = Schema(..., title='Points', min_length=1)


pd = PanelData.validate({'name': 'Dummy Panel', 'points': [{'name': 'Dummy Point', 'gios_type': DIGITAL_INPUT}]})
assert pd.dict(skip_defaults=True)['points'][0]['gios_type'] == DIGITAL_INPUT
print(pd.dict(skip_defaults=True))

{'name': 'Dummy Panel', 'points': [{'name': 'Dummy Point', 'gios_type': 'digital_input'}]}

Notice how the dict contains 'gios_type' field.

[samuelcolvin]

I agree that it should, PR welcome

[tiangolo]

@skewty in this case you are explicitly passing a value to PanelData.validate with 'gios_type': DIGITAL_INPUT.

You are passing the same value as the one that would be used by default, but you are passing it explicitly.

If you don't pass an explicit value, and let the model take the default one (the same value, but using the default), then Pydantic respects the skip_defaults:

from typing import List
from pydantic import BaseModel, Schema

DIGITAL_INPUT = 'digital_input'


class Payload(BaseModel):
    """Base class for all Combox payloads"""


class PointData(Payload):
    name: str = Schema(..., title='Name', min_length=1, max_length=32)
    gios_type: str = Schema(DIGITAL_INPUT, title='GIOS Type')


class PanelData(Payload):
    name: str = Schema(..., title='Name', min_length=1, max_length=32)
    points: List[PointData] = Schema(..., title='Points', min_length=1)


pd = PanelData.validate({'name': 'Dummy Panel', 'points': [{'name': 'Dummy Point'}]})
assert 'gios_type' not in pd.dict(skip_defaults=True)['points'][0]
print(pd.dict(skip_defaults=True))

---

I consider this behavior (the current behavior) a feature (useful), for example in this example:

Let's say you have this model:

class Person(BaseModel):
    name: str
    pet: str = None

You have an item in your DB that contains:

{"name": "alice", "pet": "orion"}

Now let's say you receive an update to that item with:

{"name": "bob"}

As pet is not set, you can keep the current value of "orion", but you can update the name to "bob". Using skip_defaults skips the non-existing "pet" that would be set to None by default.

Now in your DB:

{"name": "bob", "pet": "orion"}

Then, if you receive an update to the same item with:

{"pet": None}

You could update your item in the DB, marking the pet as dead (None).

Now in your DB:

{"name": "bob", "pet": None}

In this case, Pydantic would be smart enough to know that pet was intentionally and explicitly set to None, even when that was the default value.

It's able to distinguish between values taken by default and values set, even if the value is the same.

[skewty]

@tiangolo thank you for taking the time to make such a complete reply and sorry for taking so long to get back to this.

I consider this behavior (the current behavior) a feature (useful), for example in this example:
...
In this case, Pydantic would be smart enough to know that pet was intentionally and explicitly set to None, even when that was the default value.

It's able to distinguish between values taken by default and values set, even if the value is the same.

To me anyway, it is surprising that " It's able to distinguish between values taken by default and values set, even if the value is the same." To me, I see the feature more as a skip_unset_defaults=True rather than a skip_defaults=True because you are not really skipping default values as the parameter name suggests (to me).

That said, would an alternate parameter name be acceptable to accomplish my version be acceptable? Or perhaps some sort of additional parameter like strict=True which I would set to False to accomplish what I was looking for when I created this issue?

[skewty]

I realize changing how skip_defaults parameter works would be a breaking change and such a change would be best done before v1.0. So this issue / discussion is coming up at a good time :)

[samuelcolvin]

I'm not entirely clear what change/new feature you would like?

[dmontagu]

@samuelcolvin I think the request is to change the skip_defaults argument of BaseModel.dict to exclude values if they are equal to the default value (whether or not it was set), and to have a new keyword argument (called skip_unset_defaults; I would propose just skip_unset) that implements the existing functionality.

For what it's worth, I also found the skip_defaults behavior surprising at first; skip_unset might have been a clearer name, especially considering most other libraries wouldn't even be capable of supporting the skip_defaults feature, so I think most people wouldn't guess that's how it works. Separately, I feel like skipping defaults (whether or not they were set) is a relatively common feature request. (For example, it would make it a lot easier to skip nulls in many cases).

At this point I'm familiar and comfortable with the existing implementation, so I'm fine either way, but I can see some merits.

[samuelcolvin]

skip_defaults and skip_unset sounds reasonable to me.

One advantage is that there's no more logic required on parsing so performance (of raw parsing) would be unchanged.

My only concern is that skip_defaults has new behaviour but has a name that shadows a different old keyword argument which now has a new name. This will cause a lot of headaches when migrating to v1 since tests and setup won't fail immediately.

One solution would be to completely change the names to: exclude_defaults and exclude_unset. That way there can be no confusion with the names and it's more consistent with the exclude argument at the expensive for 3 more characters and one more syllable.

What do you think?

[dmontagu]

I like the consistency with exclude.

[samuelcolvin]

do we still want this before v1 is released?

If so it needs someone to start it in the next week or so.

[dmontagu]

I think it's worthwhile. I'll try to get to this this weekend if no one else wants to handle it sooner.

[samuelcolvin]

might require logic shared to #898 to get the default values.