From def51860e4059d22b0df7ddc3899ebcf5f1903b1 Mon Sep 17 00:00:00 2001 From: Benedikt Hegner Date: Thu, 13 Jul 2023 00:02:42 +0200 Subject: [PATCH 01/10] add regex to support unit definition --- python/podio/podio_config_reader.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/python/podio/podio_config_reader.py b/python/podio/podio_config_reader.py index 4e99e2423..b20037de6 100644 --- a/python/podio/podio_config_reader.py +++ b/python/podio/podio_config_reader.py @@ -30,6 +30,10 @@ class MemberParser: name_str = r'([a-zA-Z_]+\w*)' name_re = re.compile(name_str) + # Units are given in square brakets + unit_str = r'(?:\[([a-zA-Z_*\/]+\w*)\])?' + unit_re = re.compile(unit_str) + # Comments can be anything after // # stripping of trailing whitespaces is done later as it is hard to do with regex comment_str = r'\/\/ *(.*)' @@ -41,8 +45,8 @@ class MemberParser: def_val_str = r'(?:{(.+)})?' array_re = re.compile(array_str) - full_array_re = re.compile(rf'{array_str} *{name_str} *{def_val_str} *{comment_str}') - member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str} *{comment_str}') + full_array_re = re.compile(rf'{array_str} *{name_str} *{def_val_str} * {unit_str} *{comment_str}') + member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str} * {unit_str} *{comment_str}') # For cases where we don't require a description bare_member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str}') @@ -62,13 +66,13 @@ def _parse_with_regexps(string, regexps_callbacks): @staticmethod def _full_array_conv(result): """MemberVariable construction for array members with a docstring""" - typ, size, name, def_val, comment = result.groups() + typ, size, name, def_val, unit, comment = result.groups() return MemberVariable(name=name, array_type=typ, array_size=size, description=comment.strip(), default_val=def_val) @staticmethod def _full_member_conv(result): """MemberVariable construction for members with a docstring""" - klass, name, def_val, comment = result.groups() + klass, name, def_val, unit, comment = result.groups() return MemberVariable(name=name, type=klass, description=comment.strip(), default_val=def_val) @staticmethod From 11615b9d45f4276ae50622f4c8ace7c1c7bee8e0 Mon Sep 17 00:00:00 2001 From: Benedikt Hegner Date: Thu, 13 Jul 2023 07:53:58 +0200 Subject: [PATCH 02/10] add tests --- python/podio/generator_utils.py | 4 +++- python/podio/podio_config_reader.py | 6 +++--- python/podio/test_MemberParser.py | 7 +++++++ tests/datalayout.yaml | 8 ++++---- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/python/podio/generator_utils.py b/python/podio/generator_utils.py index 17d3a37f3..95c181f80 100644 --- a/python/podio/generator_utils.py +++ b/python/podio/generator_utils.py @@ -91,6 +91,7 @@ def __init__(self, name, **kwargs): self.full_type = kwargs.pop('type', '') self.description = kwargs.pop('description', '') self.default_val = kwargs.pop('default_val', None) + self.unit = kwargs.pop('unit', None) self.is_builtin = False self.is_builtin_array = False self.is_array = False @@ -190,7 +191,8 @@ def _to_json(self): # things again here from available information def_val = f'{{{self.default_val}}}' if self.default_val else '' description = f' // {self.description}' if self.description else '' - return f'{self.full_type} {self.name}{def_val}{description}' + unit = f'[{self.unit}]' if self.unit else '' + return f'{self.full_type} {self.name}{def_val}{unit}{description}' class DataModel: # pylint: disable=too-few-public-methods diff --git a/python/podio/podio_config_reader.py b/python/podio/podio_config_reader.py index b20037de6..2fb618681 100644 --- a/python/podio/podio_config_reader.py +++ b/python/podio/podio_config_reader.py @@ -45,8 +45,8 @@ class MemberParser: def_val_str = r'(?:{(.+)})?' array_re = re.compile(array_str) - full_array_re = re.compile(rf'{array_str} *{name_str} *{def_val_str} * {unit_str} *{comment_str}') - member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str} * {unit_str} *{comment_str}') + full_array_re = re.compile(rf'{array_str} *{name_str} *{def_val_str} *{unit_str} *{comment_str}') + member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str} *{unit_str} *{comment_str}') # For cases where we don't require a description bare_member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str}') @@ -73,7 +73,7 @@ def _full_array_conv(result): def _full_member_conv(result): """MemberVariable construction for members with a docstring""" klass, name, def_val, unit, comment = result.groups() - return MemberVariable(name=name, type=klass, description=comment.strip(), default_val=def_val) + return MemberVariable(name=name, type=klass, description=comment.strip(), unit=unit, default_val=def_val) @staticmethod def _bare_array_conv(result): diff --git a/python/podio/test_MemberParser.py b/python/podio/test_MemberParser.py index c7e4ec2bb..29e08b51d 100644 --- a/python/podio/test_MemberParser.py +++ b/python/podio/test_MemberParser.py @@ -243,6 +243,13 @@ def test_parse_valid_no_description(self): self.assertEqual(parsed.description, 'descriptions are not ignored even though they are not required') self.assertTrue(not parsed.is_builtin) + def test_parse_unit(self): + """Test that units are properly parsed""" + parser = MemberParser() + + parsed = parser.parse('unsigned long long var [GeV] // description') + self.assertEqual(parsed.unit, 'GeV') + def test_string_representation(self): """Test that the string representation that is used in the jinja2 templates includes the default initialization""" diff --git a/tests/datalayout.yaml b/tests/datalayout.yaml index cfca90388..f202cbddd 100755 --- a/tests/datalayout.yaml +++ b/tests/datalayout.yaml @@ -66,10 +66,10 @@ datatypes : Author : "B. Hegner" Members: - unsigned long long cellID // cellID - - double x // x-coordinate - - double y // y-coordinate - - double z // z-coordinate - - double energy // measured energy deposit + - double x [mm] // x-coordinate + - double y [mm] // y-coordinate + - double z [mm] // z-coordinate + - double energy [GeV] // measured energy deposit ExampleMC : Description : "Example MC-particle" From 728cd48c36a75caf1b508e98c45005e903db5128 Mon Sep 17 00:00:00 2001 From: Benedikt Hegner Date: Thu, 13 Jul 2023 09:36:50 +0200 Subject: [PATCH 03/10] add unit syntax documentation --- doc/datamodel_syntax.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/datamodel_syntax.md b/doc/datamodel_syntax.md index e665860e6..b2ca2ff44 100644 --- a/doc/datamodel_syntax.md +++ b/doc/datamodel_syntax.md @@ -85,6 +85,13 @@ It is also possible to specify default values for members via Note that in this case it is extremely expensive to check whether the provided `default-value` results in valid c++. Hence, there is only a very basic syntax check, but no actual type check, and wrong default values will be caught only when trying to compile the datamodel. +For describing physics quantities it is important to know their units. Thus it is possible to add the units to the member definition: + +```yaml + Members: + {} [] // +``` + ### Definition of references between objects: There can be one-to-one-relations and one-to-many relations being stored in a particular class. This happens either in the `OneToOneRelations` or `OneToManyRelations` section of the data definition. The definition has again the form: From a18700a5b4d2697a490e0bffd718fc36187eb998 Mon Sep 17 00:00:00 2001 From: Benedikt Hegner Date: Thu, 13 Jul 2023 11:28:10 +0200 Subject: [PATCH 04/10] extend tests for members --- python/podio/test_MemberParser.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/python/podio/test_MemberParser.py b/python/podio/test_MemberParser.py index 29e08b51d..8aace0b80 100644 --- a/python/podio/test_MemberParser.py +++ b/python/podio/test_MemberParser.py @@ -250,6 +250,11 @@ def test_parse_unit(self): parsed = parser.parse('unsigned long long var [GeV] // description') self.assertEqual(parsed.unit, 'GeV') + parsed = parser.parse('unsigned long long var{42} [GeV] // description') + self.assertEqual(parsed.unit, 'GeV') + self.assertEqual(parsed.default_val, '42') + + def test_string_representation(self): """Test that the string representation that is used in the jinja2 templates includes the default initialization""" From 3314e169f3bed74aafc5a79f6aa16ed1399c91c9 Mon Sep 17 00:00:00 2001 From: Benedikt Hegner Date: Thu, 13 Jul 2023 13:16:50 +0200 Subject: [PATCH 05/10] add a component with unit --- tests/datalayout.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/datalayout.yaml b/tests/datalayout.yaml index f202cbddd..38afb58d0 100755 --- a/tests/datalayout.yaml +++ b/tests/datalayout.yaml @@ -26,7 +26,7 @@ components : Description : "A not so simple struct" Author : "Someone" Members: - - SimpleStruct data // component members can have descriptions + - SimpleStruct data [GeV] // component members can have descriptions and units ex2::NamespaceStruct: Members: From 46890e4a032d69603947e6725d6bef35b3824f57 Mon Sep 17 00:00:00 2001 From: Benedikt Hegner Date: Wed, 19 Jul 2023 15:27:40 +0200 Subject: [PATCH 06/10] add units to generated documentation --- python/podio/generator_utils.py | 12 ++++++++++-- python/templates/macros/declarations.jinja2 | 16 ++++++++-------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/python/podio/generator_utils.py b/python/podio/generator_utils.py index 95c181f80..a27798aa5 100644 --- a/python/podio/generator_utils.py +++ b/python/podio/generator_utils.py @@ -151,6 +151,14 @@ def __init__(self, name, **kwargs): else: self.namespace, self.bare_type = _get_namespace_class(self.full_type) + @property + def docstring(self): + if self.unit is not None: + docstring = rf'{self.description} [{self.unit}]' + else: + docstring = self.description + return docstring + def __str__(self): """string representation""" # Make sure to include scope-operator if necessary @@ -164,8 +172,8 @@ def __str__(self): else: definition = rf'{scoped_type} {self.name}{{}};' - if self.description: - definition += rf' ///< {self.description}' + if self.docstring: + definition += rf' ///< {self.docstring}' return definition def getter_name(self, get_syntax): diff --git a/python/templates/macros/declarations.jinja2 b/python/templates/macros/declarations.jinja2 index 4ee9522f4..af46f499e 100644 --- a/python/templates/macros/declarations.jinja2 +++ b/python/templates/macros/declarations.jinja2 @@ -8,15 +8,15 @@ {% macro member_getters(members, get_syntax) %} {%for member in members %} - /// Access the {{ member.description }} + /// Access the {{ member.docstring }} const {{ member.full_type }}& {{ member.getter_name(get_syntax) }}() const; {% if member.is_array %} - /// Access item i of the {{ member.description }} + /// Access item i of the {{ member.docstring }} const {{ member.array_type }}& {{ member.getter_name(get_syntax) }}(size_t i) const; {%- endif %} {% if member.sub_members %} {% for sub_member in member.sub_members %} - /// Access the member of {{ member.description }} + /// Access the member of {{ member.docstring }} const {{ sub_member.full_type }}& {{ sub_member.getter_name(get_sytnax) }}() const; {% endfor %} {% endif %} @@ -27,24 +27,24 @@ {% macro member_setters(members, get_syntax) %} {% for member in members %} - /// Set the {{ member.description }} + /// Set the {{ member.docstring }} void {{ member.setter_name(get_syntax) }}({{ member.full_type }} value); {% if member.is_array %} void {{ member.setter_name(get_syntax) }}(size_t i, {{ member.array_type }} value); {% endif %} {% if not member.is_builtin %} - /// Get reference to {{ member.description }} + /// Get reference to {{ member.docstring }} {{ member.full_type }}& {{ member.name }}(); {% endif %} {% if member.sub_members %} {% for sub_member in member.sub_members %} {% if sub_member.is_builtin %} - /// Set the member of {{ member.description }} + /// Set the member of {{ member.docstring }} void {{ sub_member.setter_name(get_syntax) }}({{ sub_member.full_type }} value); {% else %} - /// Get reference to the member of {{ member.description }} + /// Get reference to the member of {{ member.docstring }} {{ sub_member.full_type }}& {{ sub_member.name }}(); - /// Set the member of {{ member.description }} + /// Set the member of {{ member.docstring }} void {{ sub_member.setter_name(get_sytnax) }}({{ sub_member.full_type }} value); {% endif %} {% endfor %} From 1e53b40cc26f560f25ae18f3a48a4e8e0856006d Mon Sep 17 00:00:00 2001 From: Benedikt Hegner Date: Wed, 19 Jul 2023 16:17:00 +0200 Subject: [PATCH 07/10] address formating check issues --- python/podio/generator_utils.py | 5 +++-- python/podio/test_MemberParser.py | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/python/podio/generator_utils.py b/python/podio/generator_utils.py index a27798aa5..6377aa09e 100644 --- a/python/podio/generator_utils.py +++ b/python/podio/generator_utils.py @@ -153,10 +153,11 @@ def __init__(self, name, **kwargs): @property def docstring(self): + """Docstring to be used in code generation""" if self.unit is not None: - docstring = rf'{self.description} [{self.unit}]' + docstring = rf'{self.description} [{self.unit}]' else: - docstring = self.description + docstring = self.description return docstring def __str__(self): diff --git a/python/podio/test_MemberParser.py b/python/podio/test_MemberParser.py index 8aace0b80..d99cd11d2 100644 --- a/python/podio/test_MemberParser.py +++ b/python/podio/test_MemberParser.py @@ -254,7 +254,6 @@ def test_parse_unit(self): self.assertEqual(parsed.unit, 'GeV') self.assertEqual(parsed.default_val, '42') - def test_string_representation(self): """Test that the string representation that is used in the jinja2 templates includes the default initialization""" From 4b635d4cd0ea660fb69644cde140197e43ccc923 Mon Sep 17 00:00:00 2001 From: Benedikt Hegner Date: Wed, 19 Jul 2023 16:33:56 +0200 Subject: [PATCH 08/10] address formating check issues --- python/podio/podio_config_reader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/podio/podio_config_reader.py b/python/podio/podio_config_reader.py index 2fb618681..abb160bc3 100644 --- a/python/podio/podio_config_reader.py +++ b/python/podio/podio_config_reader.py @@ -67,7 +67,7 @@ def _parse_with_regexps(string, regexps_callbacks): def _full_array_conv(result): """MemberVariable construction for array members with a docstring""" typ, size, name, def_val, unit, comment = result.groups() - return MemberVariable(name=name, array_type=typ, array_size=size, description=comment.strip(), default_val=def_val) + return MemberVariable(name=name, array_type=typ, array_size=size, description=comment.strip(), unit=unit, default_val=def_val) @staticmethod def _full_member_conv(result): From faca42ca6e1a3fd8acc6d6cdf7fcbaf211f6c9c1 Mon Sep 17 00:00:00 2001 From: Benedikt Hegner Date: Wed, 19 Jul 2023 16:53:43 +0200 Subject: [PATCH 09/10] make flake8 happy after last fix made lines too ling --- python/podio/podio_config_reader.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/podio/podio_config_reader.py b/python/podio/podio_config_reader.py index abb160bc3..d40ae3178 100644 --- a/python/podio/podio_config_reader.py +++ b/python/podio/podio_config_reader.py @@ -67,7 +67,8 @@ def _parse_with_regexps(string, regexps_callbacks): def _full_array_conv(result): """MemberVariable construction for array members with a docstring""" typ, size, name, def_val, unit, comment = result.groups() - return MemberVariable(name=name, array_type=typ, array_size=size, description=comment.strip(), unit=unit, default_val=def_val) + return MemberVariable(name=name, array_type=typ, array_size=size, description=comment.strip(), + unit=unit, default_val=def_val) @staticmethod def _full_member_conv(result): From 42edf8d2907b3e3d3ecdc40153fd3f49cde6ea46 Mon Sep 17 00:00:00 2001 From: Benedikt Hegner Date: Mon, 21 Aug 2023 13:38:03 +0200 Subject: [PATCH 10/10] fixing member parsing for a definition without comment but with unit --- python/podio/podio_config_reader.py | 13 ++++++------- tests/datalayout.yaml | 8 ++++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/python/podio/podio_config_reader.py b/python/podio/podio_config_reader.py index d40ae3178..303632367 100644 --- a/python/podio/podio_config_reader.py +++ b/python/podio/podio_config_reader.py @@ -49,8 +49,8 @@ class MemberParser: member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str} *{unit_str} *{comment_str}') # For cases where we don't require a description - bare_member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str}') - bare_array_re = re.compile(rf' *{array_str} +{name_str} *{def_val_str}') + bare_member_re = re.compile(rf' *{type_str} +{name_str} *{def_val_str} *{unit_str}') + bare_array_re = re.compile(rf' *{array_str} +{name_str} *{def_val_str} *{unit_str}') @staticmethod def _parse_with_regexps(string, regexps_callbacks): @@ -60,7 +60,6 @@ def _parse_with_regexps(string, regexps_callbacks): result = rgx.match(string) if result: return callback(result) - raise DefinitionError(f"'{string}' is not a valid member definition. Check syntax of the member definition.") @staticmethod @@ -79,14 +78,14 @@ def _full_member_conv(result): @staticmethod def _bare_array_conv(result): """MemberVariable construction for array members without docstring""" - typ, size, name, def_val = result.groups() - return MemberVariable(name=name, array_type=typ, array_size=size, default_val=def_val) + typ, size, name, def_val, unit = result.groups() + return MemberVariable(name=name, array_type=typ, array_size=size, unit=unit, default_val=def_val) @staticmethod def _bare_member_conv(result): """MemberVarible construction for members without docstring""" - klass, name, def_val = result.groups() - return MemberVariable(name=name, type=klass, default_val=def_val) + klass, name, def_val, unit = result.groups() + return MemberVariable(name=name, type=klass, unit=unit, default_val=def_val) def parse(self, string, require_description=True): """Parse the passed string""" diff --git a/tests/datalayout.yaml b/tests/datalayout.yaml index 38afb58d0..06f8e3075 100755 --- a/tests/datalayout.yaml +++ b/tests/datalayout.yaml @@ -11,10 +11,10 @@ options : components : SimpleStruct: Members: - - int x - - int y - - int z - - std::array p + - int x [mm] + - int y [mm] + - int z [mm] + - std::array p [mm] # can also add c'tors: ExtraCode : declaration: "