Introduce NXemission_line base class#1619
Conversation
|
The PR now adds
|
|
From Telco Apr 29: general approval was received. There were rumblings of linking to external databases instead of having copies of things such as the periodic table, but the consensus was that this could be ok. There are implications to other base clases, e.g. NXchemical_composition which has an ELEMENT field of type NXatom, and in contributed, NXsubstance. This should be discussed more, such as should we introduce a new base class, NXelement, or should the XAS application definitions use NXatom instead? |
|
After looking at |
|
Great, @mretegan will you provide a new version then? |
|
@phyy-nx Here is a version using I only use the |
|
That looks reasonable to me. I see quite a few changes in that version of NXxas compared to the public version. What are your plans for that version of NXxas and for this PR at this point? |
e30a2fa to
0c6b066
Compare
|
@phyy-nx I apologize for dragging my feet with this PR. It should be ready for review. I am unsure whether The |
|
From Telco:
|
|
Maybe rename |
|
Howdy folks. I looked for precedent for NXemission_lines being a simple container for NXemission_line by first reading through the list of base classes and contributed definitions. Nothing immediately caught my eye. However, I think it's not needed anyway. Looking at You get this: And a NeXus file following this pattern could look like: The default I think the first version without the partial names looks cleaner though. @mretegan Does that make sense? What do you think of this pattern? |
|
@phyy-nx Using If we use @PeterC-DLS Using The idea of using Since, in general, the NX class is something that most users will not be aware of, we could go with having |
|
This is correct. NeXus base classes inside NXcollection are meaningless. The content in a NXcollection is not validated, therefore it cannot be used to define any NeXus standard. |
IMHO, |
|
Strong -1 from me on "atom". That needs to be "element". |
|
Fair enough. I see that NXatom is enough different from NXelement that they could coexist. I'm still not totally on board with a copy of the periodic table in NeXus, as in the original PR. Does |
|
The enumeration was used for validation purposes. Without it, it would need to happen later. If that is not an issue, then I could implement the following structure: |
|
Personally I like that better. Ideally we want to refer to some other ontology to validate element names against, thought that doesn't really exist yet. Not specifying it here future proofs us for when that is available. At least those are my musing. Would welcome other opinions. |
|
@mretegan @phyy-nx Element seems like the textbook example of an enumeration. How could a data structure with any string (NX_CHAR) for "symbol" (not limited to one upper case letter followed optionally by one lower case letter), a field (presumably but not clearly a number, let alone a positive integer between 1 and 200) for "atomic number", and a field (presumably but not clearly a number, let alone a positive float less than 5000.) possibly be preferable to an Enumeration for Element? Baffled. |
The PR introduces a new base class to describe an emission line. This base class will be used in the updated XAS definition (see #1352)
Because this may be of broader interest, and to make the review of the forthcoming XAS application definition easier, this base class is being submitted as a separate PR.
A question that came up in our previous discussion (XraySpectroscopy#8) was how to group together multiple emission lines.
NXcollectionwas discarded as it is not validated. Another proposed alternative wasNXobject(XraySpectroscopy#8 (comment)) or using a new container classNXemission_lines, which would result in an HDF5 structure looking like this: