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

Retighten specification of NXdata's axes attribute for #1381 #1392

Closed
wants to merge 1 commit into from

Conversation

PeterC-DLS
Copy link
Contributor

Tweak examples and clarify that alternative choices for axis values can be indicated by the presence of its corresponding AXISNAME_indices attribute

Tweak examples and clarify that alternative choices for axis values can be indicated by the presence of its corresponding AXISNAME_indices attribute
@PeterC-DLS PeterC-DLS requested review from woutdenolf and a team June 26, 2024 17:18
@PeterC-DLS
Copy link
Contributor Author

@jacobfilik, please review too.

Copy link
Contributor

@woutdenolf woutdenolf left a comment

Choose a reason for hiding this comment

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

As I mentioned in #1381 I do agree we approved #1213 without voting while it does change the standard. So we should put it back.

But I don't think this tweak is enough, that's how we ended up with an inconsistent and impossible to read NXdata doc in the first place.

I'll try to make an alternative PR that explicitly states that the length of @axes must be equal to rank of signal. And then discuss how you need to find all axes of NXdata and change the examples to show what to do with the inconsistent definition as a result of the constraint.

@woutdenolf
Copy link
Contributor

I'll try to make an alternative PR that explicitly states that the length of @axes must be equal to rank of signal. And then discuss how you need to find all axes of NXdata and change the examples to show what to do with the inconsistent definition as a result of the constraint.

#1396

@PeterC-DLS
Copy link
Contributor Author

Abandoned in favour of #1396

@PeterC-DLS PeterC-DLS closed this Aug 20, 2024
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.

3 participants