-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
No inner sections #525
No inner sections #525
Conversation
…nt as the field value
…able quantity <name>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some code cleanup, the content looks good to me.
src/pynxtools/nomad/schema.py
Outdated
for sec in entry.m_all_contents(): | ||
if isinstance(sec, ActivityStep): | ||
sec_c = sec.m_copy() | ||
self.steps.append(sec_c) | ||
ref = NexusActivityStep(name=sec.name, reference=sec) | ||
self.steps.append(ref) | ||
elif isinstance(sec, basesections.Instrument): | ||
ref = InstrumentReference(name=sec.name) | ||
ref.reference = sec | ||
ref = InstrumentReference(name=sec.name, reference=sec) | ||
self.instruments.append(ref) | ||
elif isinstance(sec, CompositeSystem): | ||
ref = CompositeSystemReference(name=sec.name) | ||
ref.reference = sec | ||
ref = CompositeSystemReference(name=sec.name, reference=sec) | ||
self.samples.append(ref) | ||
elif isinstance(sec, ActivityResult): | ||
sec_c = sec.m_copy() | ||
self.results.append(sec_c) | ||
ref = NexusActivityResult(name=sec.name, reference=sec) | ||
self.results.append(ref) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion to use a mapping
mapping = {
ActivityStep: (NexusActivityStep, self.steps),
basesections.Instrument: (InstrumentReference, self.instruments),
CompositeSystem: (CompositeSystemReference, self.samples),
ActivityResult: (NexusActivityResult, self.results),
}
for sec in entry.m_all_contents():
for cls, (ref_cls, collection) in mapping.items():
if isinstance(sec, cls):
collection.append(ref_cls(name=sec.name, reference=sec))
break
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Co-authored-by: Lukas Pielsticker <[email protected]>
Co-authored-by: Lukas Pielsticker <[email protected]>
With latest version of nomad/develop and this branch, I get a number of parsing errors for the mpes example data:
It can however be that these files indeed don't adhere to the NXmpes standard.
|
…into no_inner_sections
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -34,7 +34,7 @@ jobs: | |||
curl -LsSf https://astral.sh/uv/install.sh | sh | |||
uv pip install coverage coveralls | |||
- name: Install nomad | |||
if: "${{ matrix.python_version != '3.8'}}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think at some point we should just drop support for python<3.10, like NOMAD did. I can do this in a separate PR.
No description provided.