From b6db400aa4464aa3489e752361cf000d0b0e646b Mon Sep 17 00:00:00 2001 From: Olivier Hoenen Date: Tue, 31 Mar 2026 13:40:41 +0200 Subject: [PATCH 1/2] fix conversion 3to4 of name/identifier when identifier is empty --- imas/ids_convert.py | 22 +++++++++++++++++++ imas/test/test_ids_convert.py | 41 +++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/imas/ids_convert.py b/imas/ids_convert.py index a1707c16..ad7b0d91 100644 --- a/imas/ids_convert.py +++ b/imas/ids_convert.py @@ -456,6 +456,8 @@ def _apply_3to4_conversion(self, old: Element, new: Element) -> None: # Map DD3 name -> DD4 description if name_path not in self.old_to_new.path: self._add_rename(name_path, desc_path) + # GH#114: Also preserve name in DD4 name when identifier is empty + self.old_to_new.type_change[name_path] = _name_identifier_3to4 # Map DD3 identifier -> DD4 name if id_path in self.old_to_new.path: @@ -1154,6 +1156,26 @@ def _circuit_connections_4to3(node: IDSPrimitive) -> None: node.value = new_value +def _name_identifier_3to4(source_name: IDSBase, target_description: IDSBase) -> None: + """Handle DD3to4 name→description rename, preserving name when identifier is empty. + + GH#114: The ``name→description`` rename moves DD3 ``name`` to DD4 ``description``. + Normally DD4 ``name`` is filled by the ``identifier→name`` rename. However, when + the DD3 ``identifier`` is empty/unset, ``iter_nonempty_`` never visits it, leaving + DD4 ``name`` blank. This handler copies DD3 ``name`` to *both* DD4 ``description`` + and DD4 ``name`` when the DD3 ``identifier`` is absent. + """ + # Always copy DD3 name -> DD4 description + target_description.value = source_name.value + + # When DD3 identifier is empty, also preserve name in DD4 name + source_parent = source_name._parent + source_identifier = getattr(source_parent, "identifier", None) + if source_identifier is None or not source_identifier.value: + target_parent = target_description._parent + target_parent.name = source_name.value + + def _ids_properties_source(source: IDSString0D, provenance: IDSStructure) -> None: """Handle DD3to4 migration of ids_properties/source to ids_properties/provenance.""" if len(provenance.node) > 0: diff --git a/imas/test/test_ids_convert.py b/imas/test/test_ids_convert.py index b79fb1ba..f3fa7538 100644 --- a/imas/test/test_ids_convert.py +++ b/imas/test/test_ids_convert.py @@ -575,6 +575,47 @@ def test_4to3_name_identifier_mapping_magnetics(): assert dst.b_field_pol_probe[0].identifier == "TEST_NAME" +def test_3to4_name_identifier_empty_identifier(): + """GH#114: name must be preserved when identifier is empty.""" + factory = IDSFactory("3.40.1") + + src = factory.pf_active() + src.ids_properties.homogeneous_time = IDS_TIME_MODE_HOMOGENEOUS + src.coil.resize(2) + # Case 1: name populated, identifier empty + src.coil[0].name = "CS3U" + src.coil[0].identifier = "" + # Case 2: name populated, identifier not set at all + src.coil[1].name = "PF1" + + dst = convert_ids(src, "4.0.0") + + # name must be preserved in DD4 name (not overwritten by empty identifier) + assert dst.coil[0].name == "CS3U" + assert dst.coil[0].description == "CS3U" + + assert dst.coil[1].name == "PF1" + assert dst.coil[1].description == "PF1" + + +def test_3to4_name_identifier_both_populated(): + """GH#114: when both name and identifier are populated, identifier wins.""" + factory = IDSFactory("3.40.1") + + src = factory.pf_active() + src.ids_properties.homogeneous_time = IDS_TIME_MODE_HOMOGENEOUS + src.coil.resize(1) + src.coil[0].name = "CS3U" + src.coil[0].identifier = "COIL_ID" + + dst = convert_ids(src, "4.0.0") + + # DD3 identifier -> DD4 name + assert dst.coil[0].name == "COIL_ID" + # DD3 name -> DD4 description + assert dst.coil[0].description == "CS3U" + + def test_3to4_cocos_hardcoded_paths(): # Check for existence in 3.42.0 factory = IDSFactory("3.42.0") From a1a873f1e6378d08dfcc260f9ae28a552f087aaf Mon Sep 17 00:00:00 2001 From: Olivier Hoenen Date: Tue, 31 Mar 2026 18:31:30 +0200 Subject: [PATCH 2/2] fixups from code review --- imas/ids_convert.py | 9 +-------- imas/test/test_ids_convert.py | 30 ++++++------------------------ 2 files changed, 7 insertions(+), 32 deletions(-) diff --git a/imas/ids_convert.py b/imas/ids_convert.py index ad7b0d91..ce6a1296 100644 --- a/imas/ids_convert.py +++ b/imas/ids_convert.py @@ -1157,14 +1157,7 @@ def _circuit_connections_4to3(node: IDSPrimitive) -> None: def _name_identifier_3to4(source_name: IDSBase, target_description: IDSBase) -> None: - """Handle DD3to4 name→description rename, preserving name when identifier is empty. - - GH#114: The ``name→description`` rename moves DD3 ``name`` to DD4 ``description``. - Normally DD4 ``name`` is filled by the ``identifier→name`` rename. However, when - the DD3 ``identifier`` is empty/unset, ``iter_nonempty_`` never visits it, leaving - DD4 ``name`` blank. This handler copies DD3 ``name`` to *both* DD4 ``description`` - and DD4 ``name`` when the DD3 ``identifier`` is absent. - """ + """Preserve name when identifier is empty, see GH#114.""" # Always copy DD3 name -> DD4 description target_description.value = source_name.value diff --git a/imas/test/test_ids_convert.py b/imas/test/test_ids_convert.py index f3fa7538..1c712b75 100644 --- a/imas/test/test_ids_convert.py +++ b/imas/test/test_ids_convert.py @@ -583,37 +583,19 @@ def test_3to4_name_identifier_empty_identifier(): src.ids_properties.homogeneous_time = IDS_TIME_MODE_HOMOGENEOUS src.coil.resize(2) # Case 1: name populated, identifier empty - src.coil[0].name = "CS3U" + src.coil[0].name = "TEST_NAME" src.coil[0].identifier = "" # Case 2: name populated, identifier not set at all - src.coil[1].name = "PF1" + src.coil[1].name = "TEST_NAME2" dst = convert_ids(src, "4.0.0") # name must be preserved in DD4 name (not overwritten by empty identifier) - assert dst.coil[0].name == "CS3U" - assert dst.coil[0].description == "CS3U" + assert dst.coil[0].name == "TEST_NAME" + assert dst.coil[0].description == "TEST_NAME" - assert dst.coil[1].name == "PF1" - assert dst.coil[1].description == "PF1" - - -def test_3to4_name_identifier_both_populated(): - """GH#114: when both name and identifier are populated, identifier wins.""" - factory = IDSFactory("3.40.1") - - src = factory.pf_active() - src.ids_properties.homogeneous_time = IDS_TIME_MODE_HOMOGENEOUS - src.coil.resize(1) - src.coil[0].name = "CS3U" - src.coil[0].identifier = "COIL_ID" - - dst = convert_ids(src, "4.0.0") - - # DD3 identifier -> DD4 name - assert dst.coil[0].name == "COIL_ID" - # DD3 name -> DD4 description - assert dst.coil[0].description == "CS3U" + assert dst.coil[1].name == "TEST_NAME2" + assert dst.coil[1].description == "TEST_NAME2" def test_3to4_cocos_hardcoded_paths():