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

NXmx: add NXsource and make name in NXsource and NXinstrument required #669

Merged
merged 1 commit into from
Aug 1, 2019

Conversation

phyy-nx
Copy link
Contributor

@phyy-nx phyy-nx commented Jul 22, 2019

From HDRMX meeting at ACA 2019.

Co-authored-by: Graeme Winter graeme.winter@gmail.com

…d NXsource as copy of NXsource base class but make name required.

Co-authored-by: Graeme Winter <graeme.winter@gmail.com>
@prjemian
Copy link
Contributor

@phyy-nx: This PR is languishing. Should we merge without review from @yayahb?

@prjemian prjemian added this to the NXDL 2019.10 milestone Jul 28, 2019
@yayahjb
Copy link
Contributor

yayahjb commented Jul 28, 2019 via email

@prjemian
Copy link
Contributor

@yayahb: Thanks!

@mkoennecke
Copy link
Contributor

I agree with Herbert that making the names of the instrument and the source mandatory makes sense.

I have issues with the short_name however. First of all, that is a new attribute, IMHO, not seen before. Then which name do you really want? For example HRPT at SINQ can be described as:

  • High Resolution Powder diffracTometer
  • HRPT
    The first is the long name. Next to no one knows that. In the case of HRPT it was a byproduct of inventing the acronym. Moreover the combination HRPT together with the source name SINQ fully identifies both instrument and source.

Thus I think the short name for both source and instrument should be the content of the name fields. The extended name (which may not even exist for some instruments) could be an attribute long_name. Though I do not see that it adds value except for understanding convoluted language use in instrument naming.

@yayahjb
Copy link
Contributor

yayahjb commented Jul 29, 2019 via email

@mkoennecke
Copy link
Contributor

Alright then, I overlooked the short_name in NXsource. No more objections

@yayahjb
Copy link
Contributor

yayahjb commented Jul 30, 2019 via email

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Jul 30, 2019

Thanks folks. My intent had been to go over this in the next Telco, but if there are no further objections then maybe just merging it is fine. I'll merge it Wednesday, end of day PST?

@phyy-nx phyy-nx merged commit 681e968 into master Aug 1, 2019
@phyy-nx phyy-nx deleted the NXmx_sourcename branch August 1, 2019 00:04
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.

4 participants