Skip to content

NXstage contributed class#569

Closed
mtwharmby wants to merge 1 commit intonexusformat:masterfrom
mtwharmby:NXstage
Closed

NXstage contributed class#569
mtwharmby wants to merge 1 commit intonexusformat:masterfrom
mtwharmby:NXstage

Conversation

@mtwharmby
Copy link
Copy Markdown
Contributor

Class to describe concerted motion of a set of NXpositioners, for example a diffractometer
or a collection of piezoelectric positioners in a mirror.

Signed-off-by: Michael Wharmby michael.wharmby@diamond.ac.uk

Class to describe concerted motion of a set of NXpositioners, for example a diffractometer
or a collection of piezoelectric positioners in a mirror

Signed-off-by: Michael Wharmby <michael.wharmby@diamond.ac.uk>
Copy link
Copy Markdown
Contributor

@prjemian prjemian left a comment

Choose a reason for hiding this comment

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

  • change file name to NXstage.nxdl.xml

with this change, passes punx validation tests:

prjemian@ookhd .../definitions/contributed_definitions $ punx st NXstage.nxdl.xml 
NXstage
  description
  name
  orientation : NXtransforms
  positioner1 : NXpositioner
  positioner2 : NXpositioner
prjemian@ookhd .../definitions/contributed_definitions $ punx va NXstage.nxdl.xml 
('NXstage.nxdl.xml', ' validates')

@prjemian
Copy link
Copy Markdown
Contributor

prjemian commented Jun 7, 2017

Regarding how to use names which are not defined by the NXDL spec, there is an issue (#562) to standardise amongst the NXDL files. I recommend that you comment on that issue, then modify the NXDL file (and update this PR) accordingly. This will enable automated processing of NXDL files (such as for data file validation) to recognize when a name is allowed to vary.

@prjemian
Copy link
Copy Markdown
Contributor

prjemian commented Jun 9, 2017

@mtwharmby :
I've given you a PR with proposed changes. If you agree, merge that into this PR and I will merge here.

@mtwharmby
Copy link
Copy Markdown
Contributor Author

Thanks very much. I'll review the changes and push them in. I've been talking to @markbasham about this offline and also following from the telco I think the structure of this is going to change slightly, but will certainly include your changes. Thanks again!

Copy link
Copy Markdown
Contributor

@yayahjb yayahjb left a comment

Choose a reason for hiding this comment

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

What is the type NXtransforms? What is the relationship to NXtransformations?

@mtwharmby
Copy link
Copy Markdown
Contributor Author

It's a typo. Because I was rushing. Sorry!

Copy link
Copy Markdown
Contributor

@mkoennecke mkoennecke left a comment

Choose a reason for hiding this comment

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

NXstage is basically a NXtransformations group with NXpositioners rather then values. There is a huge overlap here and I do not see why this is needed. Would it be sufficient to add name and description fields to NXtransformations to cover this use case?

@mtwharmby
Copy link
Copy Markdown
Contributor Author

Don't have time to fix this for today's telco and I think I've not expressed what I wanted to do very well with this. Will discuss more offline with @markbasham and others and put in a fresh pull request.

@mtwharmby mtwharmby closed this Jun 28, 2017
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