nexusformat / definitions

Definitions of the NeXus Standard File Structure and Contents
https://manual.nexusformat.org/
Other
26 stars 55 forks source link

typo in NXparameters #1386

Open prjemian opened 1 month ago

prjemian commented 1 month ago

In the NXparameters definition, term should be all caps (meaning that any valid field name could be used). Also, the data type should be completely flexible (such as NX_CHAR_OR_NUMBER). https://github.com/nexusformat/definitions/blob/69f92a320f15effe29e11237fcf2cad6674f55c3/base_classes/NXparameters.nxdl.xml#L30

rayosborn commented 1 month ago

I think it's worth specifying allowed attributes of TERM as well. In NeXpy, we use "error", "initial_value", "max", "min", "vary" (i.e., True if it was fitted), and "expr". The last contains a string that the LMFIT package uses to constrain parameters by expressions involving the other parameters. This might be tricky to put in a definition, because we've had a lot of trouble defining functions in NeXus and the function here is defined by an external package (LMFIT). The variables in the expression also refer to the names of functions defined outside the group, so we would probably have to leave "expr" out, but the others are more generally valid.

prjemian commented 1 month ago

PR #1039 has been advised to use NXparameters. This change would better inform them about the as-defined term field.

prjemian commented 1 month ago

@rayosborn Is your comment about this base class in general, or specific to some application definition?

rayosborn commented 1 month ago

I think it is appropriately general to be added to the base class, but we obviously need to get some feedback from others. I guess that's what PRs are for. These attributes have been in place for many years in NeXpy, so it's possible that there have been some changes to allowed NeXus attributes that are relevant.

prjemian commented 1 month ago

Also, we should note that any and all content within a NXcollection group specified by an application definition cannot be validated. NXcollection is and will always be for unvalidated content. This should be made clear in future NIAC reviews of contributed definitions and made clear in the documentation of NXcollection, with a suggestion to use NXparameters instead.