From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C29F145C0A; Tue, 29 Oct 2024 13:51:24 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AFEB84014F; Tue, 29 Oct 2024 13:51:24 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id EAD4A40144 for ; Tue, 29 Oct 2024 13:51:22 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E8390113E; Tue, 29 Oct 2024 05:51:51 -0700 (PDT) Received: from [10.57.58.134] (unknown [10.57.58.134]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7BE203F528; Tue, 29 Oct 2024 05:51:21 -0700 (PDT) Message-ID: <998910c0-3648-4ecb-af05-181774e0d447@arm.com> Date: Tue, 29 Oct 2024 12:51:20 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 3/5] dts: use Pydantic in the configuration Content-Language: en-GB To: Dean Marx Cc: dev@dpdk.org, Honnappa Nagarahalli , =?UTF-8?Q?Juraj_Linke=C5=A1?= , Paul Szczepanek References: <20240822163941.1390326-1-luca.vizzarro@arm.com> <20240822163941.1390326-4-luca.vizzarro@arm.com> From: Luca Vizzarro In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 30/09/2024 22:45, Dean Marx wrote: > -@dataclass(slots=True, frozen=True) > +@dataclass(slots=True, frozen=True, kw_only=True, > config=ConfigDict(extra="forbid")) > > > Up to you but I think it might be worth specifying what some of these > extra pydantic args are for if we're going to keep the name of the > decorator as "dataclass." For example, this ConfigDict "forbid" argument > seems to be commonly used, same with the "before/after" modes with the > model_validator. Maybe a brief description somewhere in the docstrings, > just so others can see how it differs from the previous implementation > even without experience using pydantic. Your concern is understandable, but I don't want to risk repeating what the Pydantic docs already say. As this is a feature of Pydantic itself, it should be looked up on its documentation. On the other hand, saying why we are doing would make perfect sense, but it's also quite obvious at the same time. This is because `extra="forbid"` just means to forbid any extra fields provided (in the configuration) during deserialization. > +    @model_validator(mode="before") >      @classmethod > -    def from_dict( > -        cls, > -        entry: str | TestSuiteConfigDict, > -    ) -> Self: > -        """Create an instance from two different types. > +    def convert_from_string(cls, data: Any) -> Any: > +        """Convert the string representation into a valid mapping.""" > +        if isinstance(data, str): > +            [test_suite, *test_cases] = data.split() > +            return dict(test_suite=test_suite, test_cases=test_cases) > +        return data > > > Again this is completely your call, but might be worth explaining in the > docstrings why this "before" method is used here while the other > validators are running with "after." Similarly, this comes with an understanding of deserialization/validation/creation phases. Pydantic actually have their own meaning of "validators" which could be confusing at first glance, and may be worth reading into it. Here we are just re-using the same terminology for consistency. The validators themselves explain what they do. The phases need to be understood from Pydantic's documentation in order to understand... Pydantic code. In the code you quoted, the model doc say that instead of a mapping of fields, meaning: test_suite: hello_world test_cases: - hello_world_single_core a string could be provided: hello_world hello_world_single_core This validator is set in before mode because we can't instantiate a Pydantic model with a string, and we need to parse the string to provide the valid mappings for Pydantic. The before mode happens before the model is validated by Pydantic and the raw input is provided, the after mode happens afterwards and the instance is given instead.