Skip to content
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

Improved documentation about time stamped data #719

Merged
merged 11 commits into from
Jan 23, 2020

Conversation

mkoennecke
Copy link
Contributor

@mkoennecke mkoennecke commented Jan 23, 2020

Refs #503
I have updated the information on storing time stamped data in NeXus files. I find this now sufficiently documented. Some one else, please review and merge. It is now much better then it used to be.

@vasole
Copy link
Contributor

vasole commented Jan 23, 2020

To replace something expected to be a dataset by a group is in itself not a good idea.

Furthermore, we are replacing it by a group where everything is optional. It would have been much, much simpler for a program to test if at the same level as NXdetector/data one could find a time or timestamp or whatever name to find the additional information.

That would prevent breaking of any code.

A smarter solution, still going in the direction of what this PR implements, is to keep the dataset data as it is, and to create an NXlog inside the NXdetector where value is a link to NXdetector/data with all the required time information. You get your NXlog and nobody gets broken code.

@mkoennecke
Copy link
Contributor Author

Armando, I am sorry to disappoint you but this has been discussed and decided upon at NIAC 2016. It just took embarrassingly long to update the documentation. And anyway, you cannot use the same code for treating streaming data then for more traditional data collection schemes. Except if you adapt the code and then you can also put in the check for NXlog.

@vasole
Copy link
Contributor

vasole commented Jan 23, 2020

I agree you cannot use the same code to analyze the data, but you can use existing code to visualize the data in one case but not in the other.

@FreddieAkeroyd
Copy link
Member

Related to #503

#. *processed data*:
NeXus also defines standards for processed data. This is data which has underwent some form of data
reduction or data analysis. NeXus allows to store the results of such processing together with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allows to use can

Copy link
Contributor

@benajamin benajamin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some small language corrections.

manual/source/rules.rst Outdated Show resolved Hide resolved
manual/source/rules.rst Outdated Show resolved Hide resolved
manual/source/strategies.rst Outdated Show resolved Hide resolved
manual/source/strategies.rst Outdated Show resolved Hide resolved
manual/source/strategies.rst Outdated Show resolved Hide resolved
manual/source/strategies.rst Outdated Show resolved Hide resolved
Copy link
Member

@FreddieAkeroyd FreddieAkeroyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@FreddieAkeroyd FreddieAkeroyd merged commit 5326c9a into master Jan 23, 2020
@FreddieAkeroyd FreddieAkeroyd deleted the 503-time-stamped-data branch January 23, 2020 20:49
@vasole
Copy link
Contributor

vasole commented Jan 27, 2020

@mkoennecke

Another resolution against common sense approved in a meeting where I was not present.

How many NIAC representatives are actually developing and supporting code?

This resolution is the equivalent of changing the signature of a function instead of adding an optional argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants