-
Notifications
You must be signed in to change notification settings - Fork 74
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
silx.io.nxdata: Fixed parsing of NXcanSAS
's @uncertainties
#3657
Conversation
Fixes the bug ... |
LGTM |
I vote against supporting this. Required for generic NXdata plotting:
Add NXcanSAS requirements (irrelevant for silx):
Related issue: nexusformat/definitions#1185 |
Just in case, do not forget about the generic DATASET and DATASET_errors approach. This approach is becoming the preferred one in order not to rely on attributes (that are dangerous when using external links). |
Indeed, I have a proposal to make that part of the standard: nexusformat/definitions#1047. We already handle this in silx and h5web. |
We can revert it if you want, up to you. It would be best to have a single way to express uncertainties, but since |
|
I personally would revert this as it sets a precedent for a direction we don't want to go in. |
OK, then I'll revert |
Regarding parsing of
NXData
, this PR:@uncertainties
where a fallback was loadingsignal
's@uncertainties
: This is removed since it's nowhere in the specification and in contradiction withNXcanSAS
application definition.NXcanSAS
signal's@uncertainties
. The implementation does not check whether it's inside aNXcanSAS
application definition.