Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parse_obj() / json() not called recursively / inconsistent parse_obj() behavior #1287

Closed
skewty opened this issue Mar 4, 2020 · 8 comments
Closed

Comments

@skewty
Copy link
Contributor

skewty commented Mar 4, 2020

Bug

Output of python -c "import pydantic.utils; print(pydantic.utils.version_info())":

             pydantic version: 1.4
            pydantic compiled: False
                 install path: C:\Users\somebody\Projects\combox2\venv\Lib\site-packages\pydantic
               python version: 3.7.6 (tags/v3.7.6:43364a7ae0, Dec 19 2019, 00:42:30) [MSC v.1916 64 bit (AMD64)]
                     platform: Windows-10-10.0.17763-SP0
     optional deps. installed: []

from typing import Dict, Any
from pydantic import BaseModel, ValidationError, VERSION
import pytest

assert VERSION == '1.4'

class HeaderDataModel(BaseModel):
    class Header(BaseModel):
        src: str

    class Data(BaseModel):
        __root__: Dict[str, Any]

        @classmethod
        def parse_obj(cls, *args, **kwargs):
            print('parse_obj() called!')
            return super().parse_obj(*args, **kwargs)

        def json(self, *args, **kwargs):
            print('json() called!')
            return super().json(*args, **kwargs)

    header: Header
    data: Data


class ModelA(HeaderDataModel):
    class Header(HeaderDataModel.Header):
        extra: str

    header: Header


print('=' * 20)
print("Building ModelA with data={}.")
print("  This unexpectedly raises a ValidationError and doesn't call Data.parse_obj() or Data.json()")
with pytest.raises(ValidationError):
    """
    pydantic.error_wrappers.ValidationError: 1 validation error for ModelA
    data -> __root__
      field required (type=value_error.missing)
    """
    print(ModelA.parse_obj({'header': {'src': 'core', 'extra': 'test'}, 'data': {}}).json())
print('=' * 20)
print("Building ModelA.Data with {}")
print(ModelA.Data.parse_obj({}).json())
print('=' * 20)
print("Done")

Notice in the code snippet that parse_obj() accepts just {} for data when provided directly to the object but not when provided to an encapsulating model. This is unexpected. To further make this situation difficult, you cannot just override a method because they don't seem to be called recursively. This, to me, is also unexpected.

@skewty skewty added the bug V1 Bug related to Pydantic V1.X label Mar 4, 2020
@skewty
Copy link
Contributor Author

skewty commented Mar 4, 2020

This seems somewhat related to #1193

@samuelcolvin samuelcolvin added feature request and removed bug V1 Bug related to Pydantic V1.X labels Mar 6, 2020
@samuelcolvin
Copy link
Member

I agree it would be elegant if the same method was used for initialising models publicly and recursively creating models, I think this is something we can resolve in v2, but likely not before.


For now, instead you can modify validate which I think is called recursively.

@samuelcolvin
Copy link
Member

I don't know when you think json() should be called recursively, I can't imagine any situation.

@skewty
Copy link
Contributor Author

skewty commented Mar 6, 2020

I understand that the JSON decoder specified in Config should be able to handle the task so why take the complexity / performance hit and call each model's .json() method line of thinking.

In my case, I wanted to work around some pydantic behavior. So I took the standard approach and sub-classed to adjust how a method works. That possibility was removed. The .json() method is more of a handy shortcut to an external function. When a developer overrides a method it is expected that it will be called instead of the super()'s. This doesn't appear to be happening. That is all.

My situation is the following:

We are trying to create a pydantic.BaseModel class that will accept any Dict[str, Any] with unknown field names (or an empty dict). __root__ seemed to be the answer but due to behavior stated above it is broken for my use case.

This particular model is to be used in a WireShark type of application that watches topics on a message exchange and shows their contents in webpage. We do not want the web server to couple an ever expanding set of pydantic models from our library of modules that feed into the message exchange. In most cases we just receive a message from the exchange and forward it over to websocket clients. There is no need to do a lot of validation or anything on the data.

The complete payload goes something like this:

class MyBaseModel(BaseModel):
    __extra_field_a__ = ...
    __extra_field_b__ = ....
    def extra_method_a(): ...
    def extra_method_b(): ...

class Header(MyBaseModel):
    timestamp: float
    source: str
    destination: str

class AnyData(MyBaseModel):
    __root__: Dict[str, Any]

class BasePayload(MyBaseModel):
    header: Header
    data: MyBaseModel

class AnyDataPayload(MyBaseModel)
    data: AnyData

There are many "Data" payload classes. We don't want to couple them in this case. We just want to use AnyDataPayload instead.

@coldino
Copy link

coldino commented Apr 14, 2020

This is especially important with roots of different types, such as this:

class MinMaxRange(BaseModel):
    __root__: Tuple[float, float]
    class Config:
        title = "Range (min, max)"

This works for both json() and parse_obj() when used directly:

>>> MinMaxRange(__root__=(1,2)).json()
'[1.0, 2.0]'
>>> MinMaxRange.parse_obj([1,2])
MinMaxRange(__root__=(1.0, 2.0))

But does not work when embedded in a different object for either json() or parse_obj():

class LootCrate(BaseModel):
    levelReq: MinMaxRange
>>> LootCrate(levelReq=MinMaxRange(__root__=(1,2))).json()
'{"levelReq": {"__root__": [1.0, 2.0]}}'  # incorrect

>> LootCrate.parse_obj(dict(levelReq=(1,2)))
ValidationError: 1 validation error for LootCrate
levelReq
  value is not a valid dict (type=type_error.dict)

Aside: The motivation behind this usage is schema generation. While a Tuple could be used directly in the parent object, it would require duplication of schema metadata in every field that uses this type.

@cjkoral
Copy link

cjkoral commented Jul 1, 2020

Will there be any movement on this issue in the near future?

@PrettyWood
Copy link
Member

Related issue: #1193. Will probably be tackled in v2

@PrettyWood
Copy link
Member

Closed by #2238

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants