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

add choice element to NXDL #619

Closed
prjemian opened this issue Apr 19, 2018 · 9 comments · Fixed by #622
Closed

add choice element to NXDL #619

prjemian opened this issue Apr 19, 2018 · 9 comments · Fixed by #622

Comments

@prjemian
Copy link
Contributor

prjemian commented Apr 19, 2018

As part of adding new geometry definitions to NXdetector, it is proposed to extend the NXDL to allow for specification of multiple choices for a named group or field.

The proposed use seems clear, to create a group with a defined name that is one specific NXDL class from a list of possibilities. The immediate children of the <choice> should all be of the same type (either all <field> or all <group> elements) and all should have the same @name attribute.

@prjemian prjemian added this to the NXDL 2018.4 milestone Apr 19, 2018
@prjemian prjemian self-assigned this Apr 19, 2018
@prjemian
Copy link
Contributor Author

See the two examples of <choice> highlighted in commit a55aa9c .

@prjemian
Copy link
Contributor Author

created branch "choice_element_619" for this work

@zjttoefs
Copy link
Contributor

zjttoefs commented May 2, 2018

This has code (in a branch), so should probably go into the next milestone, if possible.

@prjemian
Copy link
Contributor Author

@matthew-d-jones , @zjttoefs , @mkoennecke

The example code shows:

<choice>
    <group name="pixel_shape" type="NXoff_geometry">...</group>
    <group name="pixel_shape" type="NXcylindrical_geometry">...</group>
</choice>

To ensure that the @name attribute is defined once, I'm going to change that syntax to:

<choice name="pixel_shape">
    <group type="NXoff_geometry">...</group>
    <group type="NXcylindrical_geometry">...</group>
</choice>

Now, the @name attribute is defined once and we know that any of the choices must have only that name.

prjemian added a commit that referenced this issue May 10, 2018
@prjemian
Copy link
Contributor Author

test the revised schema using this command:

xmllint --noout --schema nxdl.xsd contributed_definitions/NXpete.nxdl.xml

@prjemian
Copy link
Contributor Author

prjemian commented May 10, 2018

before we close this ticket, delete the test file and update the documentation

prjemian added a commit that referenced this issue May 10, 2018
@prjemian
Copy link
Contributor Author

  • end of defs_intro.rst
  • autogenerated content in nxdl_desc.rst

prjemian added a commit that referenced this issue May 10, 2018
These revisions will apply ``choice`` only to groups.  Is there any
__need__ to apply it to fields, as well?  Not sure I see the value in
that now.
prjemian added a commit that referenced this issue May 10, 2018
@zjttoefs
Copy link
Contributor

There is no need for choice for fields now. I can easily see this for the future, but I am happy to wait for the code until we cross that bridge.

The changed syntax to me isn't better or worse for this current use case. I've not looked into it in detail, but it does not less flexible. Can you only have a choice between things that are supposed to have the same name now? I can come up with a good example, but for transitioning to newer versions of the definitions you could have a choice between old_scheme and new_scheme (NXgeometry vs NXtransformation maybe), where the names don't have to be the same.
I'm mainly checking I understand it correctly, because all we want to have right now appears to work.

@prjemian
Copy link
Contributor Author

Your example of a this-or-that choice with different names is already covered by the existing NXDL Schema, isn't it? One defines the two components and makes them both optional. The existing NXDL Schema does not cover the case where we must have one or the other.

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

Successfully merging a pull request may close this issue.

2 participants