-
Notifications
You must be signed in to change notification settings - Fork 56
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
typo in NXparameters #1386
Comments
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. |
PR #1039 has been advised to use NXparameters. This change would better inform them about the as-defined |
@rayosborn Is your comment about this base class in general, or specific to some application definition? |
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. |
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. |
This has been addressed by #1402 definitions/base_classes/NXparameters.nxdl.xml Lines 30 to 33 in 9059815
|
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 asNX_CHAR_OR_NUMBER
).definitions/base_classes/NXparameters.nxdl.xml
Line 30 in 69f92a3
The text was updated successfully, but these errors were encountered: