Distributions fix

[paulotex]

captures an exception in the distributions IDS that would be triggered with some species did not have a name defined. This is particular relevant for ASTRA scenarios.

[Copilot]

Pull request overview

This PR updates the distributions ParaView plugin to avoid crashing when a distribution’s species name is missing by catching an exception during distribution-name construction (notably for ASTRA scenarios).

Changes:

• Wrap distribution name generation in a try/except to handle missing species name fields and fall back to a default name.
• Emit a warning when a distribution name cannot be derived from the IDS.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[paulotex]

@SBlokhuizen I am aware you contract is cloed, but could you have a look if this seems ok to you?
Astra scenarios don't fill the 'name' of most distribution species (ions, neutrals). Instead of an exception and a stack trace, I use the species index as a name.

[maarten-ic]

Hi Paulo, an AttributeError should only be possible when you load data in a DD version that doesn't define any of the data fields the logic is looking for. Maybe it's related to the rename from label -> name? https://imas-data-dictionary.readthedocs.io/en/latest/generated/ids/distributions.html#distributions-distribution-species-ion-name

It'd be better to handle that case in the _create_dist_name function instead of wrapping it in a try-except at the call side IMO

[paulotex]

Hi @maarten-ic thanks for the quick update.
Indeed, this is data with DD 3.38, which has label but not name.
Why do you think it is better to handle this in _create_dist_name?
I prefer to not start adding code to look for label instead of name, that is, I prefer to not add code that tries to handle old DD versions.
I was hoping that with this minimal "fix", old DD versions could still be read, although losing some of the metadata (the label in this case).

[paulotex]

@maarten-ic
Ok, I just added some lines in _create_dis_name that tries name and then label. Does this look better to you? Would you do it differently perhaps?

[maarten-ic]

Hi Paulo,

Why do you think it is better to handle this in _create_dist_name?

Because the method could (though it's probably not right now) be used from multiple places, therefore it's better to handle the exception inside the method. Regardless of the solution 😉

I prefer to not start adding code to look for label instead of name, that is, I prefer to not add code that tries to handle old DD versions. I was hoping that with this minimal "fix", old DD versions could still be read, although losing some of the metadata (the label in this case).

IMAS-ParaView already does this in multiple places, so for consistency I'd handle it here as well. See, for example,

IMAS-ParaView/imas_paraview/ids_util.py

Lines 144 to 150 in 2fba6cd

	# Check if node has a name
	elif hasattr(node, "name"):
	name_appendix = str(node.name).strip()
	# Check if node has a label
	elif hasattr(node, "label"):
	name_appendix = str(node.label.value).strip()

(Thanks @SBlokhuizen )

[maarten-ic]

@maarten-ic Ok, I just added some lines in _create_dis_name that tries name and then label. Does this look better to you? Would you do it differently perhaps?

Looks fine to me!

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (5)

imas_paraview/plugins/distributions.py:174

• Same fallback pattern here can be simplified with getattr to avoid exception-driven control flow and to handle absent state/name/label consistently.

                try:
                    state_name = species.ion.state.name
                except AttributeError: # DD3 has label instead of name
                    state_name = species.ion.state.label
                result += f" State ({state_name})"

imas_paraview/plugins/distributions.py:181

• Prefer getattr-based fallback over try/except for neutral naming as well; it reduces duplication and avoids relying on AttributeError for normal DD3/DD4 differences.

            try:
                neutral_name = species.neutral.name
            except AttributeError: # DD3 has label instead of name
                neutral_name = species.neutral.label
            result = f"Neutral ({neutral_name})"

imas_paraview/plugins/distributions.py:188

• Same as above: simplify the neutral state name fallback with getattr to avoid exception-driven branching and handle missing fields consistently.

                try:
                    state_name = species.neutral.state.name
                except AttributeError: # DD3 has label instead of name
                    state_name = species.neutral.state.label
                result += f" State ({state_name})"

imas_paraview/plugins/distributions.py:166

• This change adds DD3 .label fallbacks, but the existing unit test coverage (imas_paraview/tests/test_distributions.py:test_distribution_name) only exercises the DD4 .name path. Add a test that constructs a distribution where ion/neutral use .label (and .name is absent) to prevent regressions.

            try:
                ion_name = species.ion.name
            except AttributeError: # DD3 has label instead of name
                ion_name = species.ion.label

imas_paraview/plugins/distributions.py:167

• Using try/except for attribute fallbacks makes this harder to read/maintain and still doesn’t handle missing/None values cleanly. Prefer getattr-based fallback (name -> label -> a safe default) without exceptions.

This issue also appears in the following locations of the same file:

• line 163
• line 170
• line 177
• line 184

            try:
                ion_name = species.ion.name
            except AttributeError: # DD3 has label instead of name
                ion_name = species.ion.label
            result = f"Ion ({ion_name})"

[SBlokhuizen]

Hi Paulo, the changes look fine to me! The CI fails because there is an linting/formatting issue, you can check the issue locally by running the following:

ruff format imas_paraview       # Format code
ruff check imas_paraview        # Lint code

You can read more about this in the documentation. If you need any help, please let me know!

[paulotex]

Thanks, this has been very helpful. I made some minor changes to make it more readable. I removed the try/except and used hasattr, and used a helper function to select either DD4 name or DD3 label. If you agree this looks ok (and ruff and lint are happy) I'll merge.

[maarten-ic]

Thanks Paulo! This looks ok to me, though ruff format has some complaints. Can you run it and push the changes (see Sebbe's post above)? Feel free to send one of us a message on Slack if you need any help!