[GH#71] Take into account identifier aliases #77

dee-mew · 2025-10-07T16:44:42Z

Updates in imas source and test files for handling alias information for identifiers as per issue #71.

…nto identifier_aliases

…entifier-aliases' into identifier_aliases

docs/source/identifiers.rst

maarten-ic

Hi @dee-mew, @olivhoenen,

A little more comments than I'd have liked, sorry 😅 Let me know if anything's unclear, feel free to send me a message on Slack and/or set up a short call to discuss 🙂

docs/source/identifiers.rst

imas/test/test_identifiers.py

imas/ids_identifiers.py

…nto identifier_aliases

maarten-ic · 2025-10-14T08:33:25Z

Hi @dee-mew,

Sorry for taking over this PR and pushing to your branch :)

Couple of comments why I changed things:

You can make it a lot easier for yourself by restricting the aliases argument (in IDSIdentifier.__init__) to a list. This removes quite a bit of parsing code (you're already parsing strings in _from_xml! 🙂)
IDSIdentifier.alias didn't exist before this PR, no need to keep it for backwards compatibility
Aliases / names are guaranteed to be unique -- this is checked in the Data Dictionary CI pipeline, so we don't need to check that when constructing the identifier
I've added a test with custom identifier XML to test that multiple aliases works correctly

dee-mew · 2025-10-14T09:06:01Z

Hi @dee-mew,

Sorry for taking over this PR and pushing to your branch :)

Couple of comments why I changed things:

You can make it a lot easier for yourself by restricting the aliases argument (in IDSIdentifier.__init__) to a list. This removes quite a bit of parsing code (you're already parsing strings in _from_xml! 🙂)

IDSIdentifier.alias didn't exist before this PR, no need to keep it for backwards compatibility

Aliases / names are guaranteed to be unique -- this is checked in the Data Dictionary CI pipeline, so we don't need to check that when constructing the identifier

I've added a test with custom identifier XML to test that multiple aliases works correctly

Thanks for the update @maarten-ic

maarten-ic

Code looks okay now.

My only reservation is that most tests are skipped now (because imas-data-dictionaries 4.1.0 is not yet released).
@olivhoenen how do you wish to proceed here? We could wait until DD4.1.0 is released before merging this PR, or test this again after 4.1.0 is released and fix any issues that may be present?

olivhoenen · 2025-10-16T19:07:33Z

Code looks okay now.

My only reservation is that most tests are skipped now (because imas-data-dictionaries 4.1.0 is not yet released). @olivhoenen how do you wish to proceed here? We could wait until DD4.1.0 is released before merging this PR, or test this again after 4.1.0 is released and fix any issues that may be present?

Let's wait for 4.1.0, it shall come quickly now 🤞

prasad-sawantdesai · 2025-10-21T12:50:37Z

docs/source/identifiers.rst

    False
+    >>> # Alias comparison example with materials identifier
+    >>> mid = imas.identifiers.materials_identifier
+    >>> materials = imas.IDSFactory().materials()


materials ids is not present in the data dictionary. Could you use wall IDS which uses materials_indentfier.

prasad-sawantdesai · 2025-10-21T12:52:02Z

docs/source/identifiers.rst

+    print(f"Same object: {uranium235_by_name is uranium235_by_attr}")
+    
+    # When assigning to IDS structures, any alias works the same way
+    materials = imas.IDSFactory().materials()


materials ids is not present in the data dictionary. Could you use wall IDS which uses materials_indentfier.

prasad-sawantdesai · 2025-10-21T12:56:44Z

imas/test/test_identifiers.py

+    mid = identifiers.materials_identifier
+
+    # Create an actual IDS structure
+    factory = IDSFactory("4.0.0").camera_x_rays()


We could also add example of identifiers where it is used an arrays. Aliases are just example I added for test

import imas mid = imas.identifiers.materials_identifier wallids = imas.IDSFactory().wall() wallids.description_ggd.resize(1) wallids.description_ggd[0].material.resize(1) wallids.description_ggd[0].material[0].grid_subset.resize(1) names = wallids.description_ggd[0].material[0].grid_subset[0].identifiers.names indices = wallids.description_ggd[0].material[0].grid_subset[0].identifiers.indices descriptions = wallids.description_ggd[0].material[0].grid_subset[0].identifiers.descriptions names.extend([""] * 5) indices.resize(5) descriptions.extend([""] * 5) names[0]= mid["235U"].name names[1]= mid["238U"].name names[2]= mid["U_235"].name names[3]= mid["67FGDFD"].name names[4]= mid["GFHF34"].name indices[0]= mid["235U"].index indices[1]= mid["238U"].index indices[2]= mid["U_235"].index indices[3]= mid["67FGDFD"].index indices[4]= mid["GFHF34"].index descriptions[0]= mid["235U"].description descriptions[1]= mid["238U"].description descriptions[2]= mid["U_235"].description descriptions[3]= mid["67FGDFD"].description descriptions[4]= mid["GFHF34"].description imas.util.inspect(wallids.description_ggd[0].material[0].grid_subset[0].identifiers)

prasad-sawantdesai · 2025-10-21T13:07:46Z

imas/test/test_identifiers.py

+    # Test equality with canonical name
+    factory1 = IDSFactory("4.0.0").camera_x_rays()
+    mat1 = factory1.filter_window.material
+    mat1.name = "235U"


Should assignment come from identifiers and assert later?

mat1.name = mid.U_235.name mat1.index = mid.U_235.index mat1.description = mid.U_235.description

prasad-sawantdesai · 2025-10-21T13:15:02Z

docs/source/identifiers.rst

    print(csid.total.description)

+    # Access identifiers with aliases (when available)
+    mid = imas.identifiers.materials_identifier


Should we always use name attribute? In the IDS it stored as str('materials_identifier.235U')

import imas mid = imas.identifiers.materials_identifier factory2 = imas.IDSFactory().camera_x_rays() mat2 = factory2.filter_window.material mat2.name = mid.U_235 # Should we always use name attribute? mat2.index = mid.U_235.index mat2.description = mid.U_235.description imas.util.inspect(mat2) ╭───── IDS structure: filter_window/material (DD version 4.0.1.dev324+g5180c8270.d20251021) ──────╮ │ Material of the filter window │ │ ╭──────────────────────────────────────── Attributes ─────────────────────────────────────────╮ │ │ │ has_value = True │ │ │ │ metadata = <IDSMetadata for 'material'> │ │ │ ╰─────────────────────────────────────────────────────────────────────────────────────────────╯ │ │ ╭──────────────────────────────────────── Child nodes ────────────────────────────────────────╮ │ │ │ description = <IDSString0D (IDS:camera_x_rays, filter_window/material/description, STR_0D)> │ │ │ │ str('Uranium 235 isotope') │ │ │ │ index = <IDSInt0D (IDS:camera_x_rays, filter_window/material/index, INT_0D)> │ │ │ │ int(20) │ │ │ │ name = <IDSString0D (IDS:camera_x_rays, filter_window/material/name, STR_0D)> │ │ │ │ str('materials_identifier.235U') │ │ │ ╰─────────────────────────────────────────────────────────────────────────────────────────────╯ │ ╰─────────────────────────────────────────────────────────────────────────────────────────────────╯

prasad-sawantdesai · 2025-10-21T13:16:04Z

docs/source/identifiers.rst

    core_sources.source[0].identifier = 1

+    # Identifiers can still be assigned with the old alias name for backward compatibility:
+    materials = imas.IDSFactory().materials()


materials ids is not present in the data dictionary. Could you use wall IDS which uses materials_indentfier.

prasad-sawantdesai · 2025-10-21T13:21:34Z

imas/test/test_identifiers.py

+    # Create an actual IDS structure
+    factory = IDSFactory("4.0.0").camera_x_rays()
+    mat = factory.filter_window.material
+    mat.name = "235U"


Should we add setter methods for setting name, index and description ?
for example
set_identifier("camera_x_rays.filter_window.material", "235U")
set_identifiers(wall.description_ggd[0].material[0].grid_subset[0].identifiers.names, ["235U", "U_238"]

This already exists by assigning to the structure instead of a specific name/description/index node: https://imas-python.readthedocs.io/en/stable/identifiers.html#assigning-identifiers-in-imas-python

Got it, thanks

For array of identifiers like this, do we need to use like this?
https://github.com/iterorganization/IMAS-Python/pull/77/files/ce507975c8fc1937d6e4507bdb7826cfc14197c6#r2448137366

wall.description_ggd[0].material[0].grid_subset[0].identifiers

I think this is the only place in the DD that has an array of identifiers, and it's not used in any ITER dataset at the moment.

Unless there's a good use case, I wouldn't really bother to implement something for arrays of identifiers 🙂

There are a few SOLPS cases that are currently handled within the Fortran code. For now, we’ll wait until there is a use case to use it in the Python code.

prasad-sawantdesai and others added 9 commits October 1, 2025 16:50

implementation of alias introduction in data dictionary identifiers

072a058

formatted ids_identifiers.py

ce23a41

updated test case for identifiers

71feb5c

Updated testcases in test_identifiers

81418a1

Updated testcases in test_identifiers

cd2da9d

added documentation

99e5dce

Merge branch 'identifier_aliases' of github.com:dee-mew/IMAS-Python i…

ce95f99

…nto identifier_aliases

Merge remote-tracking branch 'upstream/issue-123-Take-into-account-id…

a752965

…entifier-aliases' into identifier_aliases

Updated documentation for identifier aliases

b7d6dbe

dee-mew requested review from maarten-ic, olivhoenen and prasad-sawantdesai as code owners October 7, 2025 16:44

olivhoenen requested changes Oct 8, 2025

View reviewed changes

docs/source/identifiers.rst Outdated Show resolved Hide resolved

docs/source/identifiers.rst Outdated Show resolved Hide resolved

docs/source/identifiers.rst Outdated Show resolved Hide resolved

docs/source/identifiers.rst Outdated Show resolved Hide resolved

maarten-ic reviewed Oct 8, 2025

View reviewed changes

olivhoenen linked an issue Oct 8, 2025 that may be closed by this pull request

Take into account identifier aliases #71

Open

olivhoenen added this to the 2.0.2 milestone Oct 11, 2025

dee-mew and others added 6 commits October 14, 2025 09:34

Added functionality for multiple aliases as a list

beb6f79

Added functionality for multiple aliases as a list

698cf9a

Merge branch 'identifier_aliases' of github.com:dee-mew/IMAS-Python i…

235aeee

…nto identifier_aliases

resolved lint error for unused library

2bdb6b8

Simplify identifiers aliases and add test with multiple aliases

3c1eec3

Remove double enum import

340ea33

Updated documentation

ce50797

maarten-ic approved these changes Oct 14, 2025

View reviewed changes

olivhoenen self-requested a review October 16, 2025 19:04

prasad-sawantdesai reviewed Oct 21, 2025

View reviewed changes

prasad-sawantdesai requested changes Oct 21, 2025

View reviewed changes

[GH#71] Take into account identifier aliases #77

Are you sure you want to change the base?

[GH#71] Take into account identifier aliases #77

Uh oh!

Conversation

dee-mew commented Oct 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maarten-ic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maarten-ic commented Oct 14, 2025

Uh oh!

dee-mew commented Oct 14, 2025

Uh oh!

maarten-ic left a comment

Choose a reason for hiding this comment

Uh oh!

olivhoenen commented Oct 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants