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 BE4E043F47; Mon, 29 Apr 2024 18:15:20 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 95CE5402A3; Mon, 29 Apr 2024 18:15:18 +0200 (CEST) Received: from mail-pl1-f171.google.com (mail-pl1-f171.google.com [209.85.214.171]) by mails.dpdk.org (Postfix) with ESMTP id 2D19F4029C for ; Mon, 29 Apr 2024 18:15:17 +0200 (CEST) Received: by mail-pl1-f171.google.com with SMTP id d9443c01a7336-1e65b29f703so41159725ad.3 for ; Mon, 29 Apr 2024 09:15:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1714407316; x=1715012116; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=hWJzqv4eq/KCVrZiHOCNPwEUtz9uYhbqZRLRyXw9P+o=; b=avyNoxRVPc99PdFLz9z8vRbDXOu2kq2pq8vzgGJr/E6vIS2YZtiMkWjZnW1j2g0phx q0eZhGfyGt1FoJu18UeTgB6ePTYJq/79gVWmed3dZRGqmnHv1zj9nCbU7bXvG0EgWeb2 OULwZDbbsnjfdk588HtygR8V/W7BQyCwRSwZM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714407316; x=1715012116; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=hWJzqv4eq/KCVrZiHOCNPwEUtz9uYhbqZRLRyXw9P+o=; b=ZjH0+MWozjbbKRW1O4/b1E9vwQ/5jt6k+GHj165QY0rF1DmO+WCMP03NOB4HTOV7PN KT8nipG6AADe4OKL7afDoO9Yaqr3GldWT43tgmztNQLrl6bF8I5wcPTERVxmNJL7AzgS PkU00FiNBehl8rD2ICQc0dgFOmWp0KzGTIAWWxr0MUDnBJrUTQMQJateoalYhGamEzxF bmNLdoCuB0RH9VUP9TkMhATalC67kuUH1kL2PVmlyn38bONl0SQaRFuQQARFTPhEMHYG ikmmDL9JVZJ9bJ3cySApe5i8lUKAD5/oJ/ujOKC7pxpIRBlAGSODmnVU4U2hrqHFVlaD RBAA== X-Gm-Message-State: AOJu0YxL97xx7Wef5GXwMVif/jPyOnWr/cMDDTn20gfvR4M3leJFudX7 j/AeEY8DGLOUEFsIP+jjLKgTo/KWygeXsg6bi41vo/EWChhJrn0N1NTSEcRlqdRtffrxv0DSpO2 EEM7so2OMRdhWFM7UFMldTSRHda9cOiL86mFg0cb6WmCtXirZ6zc= X-Google-Smtp-Source: AGHT+IG4SLk02e05NrFOUv63fxAtlThs3Fzx4x8UqIfkh3jq8szdDbW7f417+uZLMjJQLefjpi1aV29UphQOR8W0DFg= X-Received: by 2002:a17:902:d509:b0:1e9:2d03:7c5d with SMTP id b9-20020a170902d50900b001e92d037c5dmr13120206plg.47.1714407316118; Mon, 29 Apr 2024 09:15:16 -0700 (PDT) MIME-Version: 1.0 References: <20240412111136.3470304-1-luca.vizzarro@arm.com> <20240412111136.3470304-4-luca.vizzarro@arm.com> In-Reply-To: <20240412111136.3470304-4-luca.vizzarro@arm.com> From: Jeremy Spewock Date: Mon, 29 Apr 2024 12:15:05 -0400 Message-ID: Subject: Re: [PATCH 3/5] dts: add parsing utility module To: Luca Vizzarro Cc: dev@dpdk.org, =?UTF-8?Q?Juraj_Linke=C5=A1?= , Paul Szczepanek Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 Fri, Apr 12, 2024 at 7:11=E2=80=AFAM Luca Vizzarro wrote: > > @@ -0,0 +1,147 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright(c) 2024 Arm Limited > + > +"""Parsing utility module. > + > +This module provides :class:`~TextParser` which can be used to model any= data structure > +that can parse a block of text. > +""" > + It would be helpful if this top level docstring explained more of how to use the text parser and some examples of using a small dataclass that chains some of these methods together. At first glance it wasn't clear to me why things were done the way they were or what these methods really provided, but looking at how you use it in the testpmd shell made more sense. > +from dataclasses import dataclass, fields, MISSING > +import re > +from typing import TypeVar > +from typing_extensions import Self > + > +T =3D TypeVar("T") This is declared but I don't see where it is used. There aren't many typehints in this file, since these types can essentially be anything, maybe using this could make some things slightly more clear (like, for example, in eq v1 and v2 are the same type) but in most cases it's implied so I'm not sure how beneficial this would be regardless. > + > + > +META_PARSERS =3D "parsers" > + > + > +def chain(parser, metadata): > + """Chain a parser function. > + > + The parser function can take and return a single argument of any typ= e. It is > + up to the user to ensure that the chained functions have compatible = types. > + > + Args: > + parser: the parser function pointer > + metadata: pre-existing metadata to chain if any > + """ > + parsers =3D metadata.get(META_PARSERS) or [] > + parsers.append(parser) > + return {**metadata, META_PARSERS: parsers} > + > + > +def to_int(metadata=3D{}, base=3D0): Is it simpler to default this to base 10? I assume that's what it'll be most of the time so we might as well allow users to skip this parameter. > + """Converts a string to an integer. > + > + Args: > + metadata: pre-existing metadata to chain if any > + base: argument passed to the constructor of ``int`` > + """ > + return chain(lambda v: int(v, base), metadata) > + > +@dataclass > +class TextParser: > + """Helper abstract dataclass that parses a text according to the fie= lds' rules. > + > + This class is accompanied by a selection of parser functions and a g= eneric chaining function, > + that are to be set to the fields' metadata, to enable parsing. If a = field metadata is not set with > + any parser function, this is skipped. > + """ > + > + @classmethod > + def parse(cls, text: str) -> Self: > + """The parsing class method. > + > + This function loops through every field that has any parser func= tion associated with it and runs > + each parser chain to the supplied text. If a parser function ret= urns None, it expects that parsing > + has failed and continues to the next field. > + > + Args: > + text: the text to parse > + Raises: > + RuntimeError: if the parser did not find a match and the fie= ld does not have a default value > + or default factory. > + """ > + fields_values =3D {} > + for field in fields(cls): > + parsers =3D field.metadata.get(META_PARSERS) > + if parsers is None: > + continue > + > + field_value =3D text > + for parser_fn in parsers: > + field_value =3D parser_fn(field_value) > + if field_value is None: > + # nothing was actually parsed, move on > + break > + > + if field_value is None: > + if field.default is MISSING and field.default_factory is= MISSING: > + raise RuntimeError( > + f"parsers for field {field.name} returned None, = but the field has no default" > + ) If we just skip instead of raising an exception here, would this solve the issues caused by the first and last line in the testpmd output? The check to see if the first line is an invalid port would obviously still not work, but would it solve the problem of the trailing prompt? > + else: > + fields_values[field.name] =3D field_value > + > + return cls(**fields_values) > -- > 2.34.1 >