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

Fix documentation of NXaperture #1341

Merged
merged 1 commit into from
Feb 8, 2024
Merged

Fix documentation of NXaperture #1341

merged 1 commit into from
Feb 8, 2024

Conversation

phyy-nx
Copy link
Contributor

@phyy-nx phyy-nx commented Jan 18, 2024

  • NXaperture was marked as deprecated but it is not, so fix the documentation
  • Explicitly add an unnamed NXoff_geometry field, the existence of which was implied by the GEOMETRY deprecation

Fixes #1231

- NXaperature was marked as deprecated but it is not, so fix the documentation
- Explicitly add an unammed NXoff_geometry field, the existence of which was implied by the GEOMETRY deprecation
@phyy-nx
Copy link
Contributor Author

phyy-nx commented Jan 18, 2024

Hi all, reviewing NXaperture more today I noticed that the NXtransformations and (deprecated) NXgeometry fields are unnamed, so I thought it reasonable that we add NXoff_geometry as unnamed as well, instead of using the name SHAPE like we did on the Telco. Thoughts? @mkoennecke

Also, it's possible since the deprecated field NXgeometry implies that there should be an NXoff_geometry field, that this PR is only a documentation fix, and therefore doesn't require a vote:

GEOMETRY: (optional) NXgeometry
DEPRECATED: Use the field depends_on and NXtransformations to position the aperture and NXoff_geometry to describe its shape

I'll wait for any feedback on this PR, but unless I hear otherwise, I'll start a vote tomorrow.

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Jan 23, 2024

Hello, please vote by providing an emoji on this comment. Thanks.

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Feb 6, 2024

Vote has passed. Needs one approving review before it can be merged @nexusformat/developers. Thanks.

@phyy-nx phyy-nx added this to the NXDL 2023.10 milestone Feb 6, 2024
Copy link
Contributor

@PeterC-DLS PeterC-DLS left a comment

Choose a reason for hiding this comment

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

LFTM

@phyy-nx phyy-nx merged commit 8f5c7e8 into main Feb 8, 2024
4 checks passed
@phyy-nx phyy-nx deleted the aperture_fix branch February 8, 2024 18:58
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.

Apparent advice loop: NXaperture and NXslit
2 participants