pydantic icon indicating copy to clipboard operation
pydantic copied to clipboard

Field(include={"__all__" ... exclude all attributes from parent entry

Open artofhuman opened this issue 3 years ago • 3 comments

Checks

Hi, I found strange behaviour in v.1.9.1, pls see code example

  • [x] I added a descriptive title to this issue
  • [x] I have searched (google, github) for similar issues and couldn't find anything
  • [x] I have read and followed the docs and still think this is a bug

Bug

    from pydantic import BaseModel, Field

    class BaseUser(BaseModel):
        id: int
        first_name: str
        last_name: str


    class OrgUser(BaseUser):
        phone: str


    class Org(BaseModel):
        id: int
        users: list[OrgUser] = Field(include={"__all__": {"first_name", "phone"}})


    users = {"id": 1, "first_name": "test 1", "last_name": "test 2", "phone": "123123"}

    org = Org(id=1, users=[users])

My expectation for org.json() is '{"id": 1, "users": [{"first_name": "test 1", "phone": "123123"}]}'

But in result '{"users": [{"first_name": "test 1", "phone": "123123"}]}'

In result, I see only that fields which were configured with Field class

artofhuman avatar Jul 24 '22 10:07 artofhuman

Confirmed that is weird. Thanks for reporting.

Could you try to fix it? Otherwise, I'll try to fix for V1.10, otherwise it'll definitely be fixed for V2.

samuelcolvin avatar Jul 24 '22 11:07 samuelcolvin

I'll try to have a look, thanks.

artofhuman avatar Jul 26 '22 04:07 artofhuman

I'm looking for an issue to contribute on, and took a look at this one

This behavior is actually not just related to "all" or parent\child models

consider the following example

class Model(BaseModel):
     arg1: int
     arg2: int = Field(include=True)

 m = Model(arg1=1, arg2=2)
 print(m.dict())

output is actually {'arg2': 2}

This behavior is actually equivalent to not defining the include in the field level and call print(m.dict(include={"arg2"})) which makes it a bit more clear as to why this behavior occurs

once a Field is defined with an include, than while exporting it, it actually treats all other fields as not to include

more specifically, this happens due to this if https://github.com/samuelcolvin/pydantic/blob/master/pydantic/main.py#L814, if not Field include was defined, then the if evaluates to False and everything is behaving as expected, but in case at least 1 Field include was defined, then if evaluates to True, and the result of the ValueItems.merge is in fact only that field

So, once the user field was defined with an include, then all other fields of that model are treated as not to include. I'm not sure this is in fact a bug and what is the expected behavior

danielkatzan avatar Jul 30 '22 16:07 danielkatzan

When call model.json or model.dict without include params, it using self.__include_fields__ and origin fields to calculate final export keys.

class Org(BaseModel):
        id: int
        users: list[OrgUser] = Field(include={"__all__": {"first_name", "phone"}})

the __include_fields__ of Org is {'users': {'__all__': {"first_name", "phone"}}},the origin fields is dict_keys(['id', 'users']), so the final export keys are {'users'}

@samuelcolvin i think maybe is not a bug.

@artofhuman you can change code to

class Org(BaseModel):
        id: int = Field(include={'id': True})
        users: list[OrgUser] = Field(include={"__all__": {"first_name", "phone"}})

users = {"id": 1, "first_name": "test 1", "last_name": "test 2", "phone": "123123"}

org = Org(id=1, users=[users])
org.dict() # {'id': 1, 'users': [{'first_name': 'test 1'}]}

fatelei avatar Sep 25 '22 10:09 fatelei

Thanks for providing the workaround.

I still don' think this is very intuitive.

I don't think setting Field(include= on users should alter whether id is included.

Hopefully this can be fixed in V2.

samuelcolvin avatar Sep 25 '22 10:09 samuelcolvin

@samuelcolvin i suggest a little fix: https://github.com/pydantic/pydantic/blob/1a5b56e1b2020fd74800064d038becf614d64a61/pydantic/main.py#L897

change

keys &= include.keys()

to

keys |= include.keys()

fatelei avatar Sep 25 '22 14:09 fatelei

we're definitely not making changes like that in V1.10 since the potential impact could be very subtle and quite severe, we need to think about this whole thing again for V2. See #4456.

samuelcolvin avatar Sep 25 '22 14:09 samuelcolvin

ok

fatelei avatar Sep 25 '22 14:09 fatelei

We’re no longer actively developing Pydantic V1 (although it will continue to receive security fixes for the next year or so), so we’re closing issues solely related to V1. If you really think this shouldn’t be closed, please comment or create a new issue 🚀.

sydney-runkle avatar Dec 06 '23 19:12 sydney-runkle