Skip to content

fix atom_types parsing#723

Merged
lukaspie merged 2 commits intomasterfrom
583-update-chemical_formula-normalization
Nov 12, 2025
Merged

fix atom_types parsing#723
lukaspie merged 2 commits intomasterfrom
583-update-chemical_formula-normalization

Conversation

@lukaspie
Copy link
Collaborator

This fixes the additional issue described in #583 (comment). The problem was that

(atom_types := sample.get("atom_types__field"))

does not actually return the value of sample.atom_types__field, but just the string atom_types__field. So we need to use the workaround presented here.

Local script for checking (using nomad==1.4 and pynxtools from this branch):

from nomad.datamodel import EntryArchive
from nomad.utils import get_logger
from pynxtools.nomad.parser import NexusParser

archive = EntryArchive()
NexusParser().parse(str("output.nxs"), archive, get_logger(__name__))

assert archive.data.ENTRY[0].specimen.atom_types__field == "C, Cr, Cu, O, Si"
assert archive.results.material.elements == ["C", "Cr", "Cu", "O", "Si"]

Here's the result section when running the GUI and appworker locally using the versions above:

image

@mkuehbach please test locally.

@lukaspie lukaspie linked an issue Nov 12, 2025 that may be closed by this pull request
@lukaspie
Copy link
Collaborator Author

lukaspie commented Nov 12, 2025

We unfortunately do not have a test for this at the moment (deactivated at https://github.com/FAIRmat-NFDI/pynxtools/blob/master/tests/nomad/test_parsing.py#L222) as atom_types is not part of NXsample and we do not have a NeXus file in the repo that uses an application definition where atom_types is defined. Could potentially add an APM/MPES file where this is used for testing.

@lukaspie lukaspie requested a review from mkuehbach November 12, 2025 16:26
Copy link
Collaborator

@mkuehbach mkuehbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix makes sense.

(in the original version atom_types was always instantiated irrespective whether the if branch is taken --- eventually to such that atom_types ends up as None)

@lukaspie lukaspie merged commit 581f6ce into master Nov 12, 2025
17 checks passed
@lukaspie lukaspie deleted the 583-update-chemical_formula-normalization branch November 12, 2025 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update chemical_formula normalization

2 participants