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

Detector channels #1252

Merged
merged 24 commits into from
Dec 20, 2023
Merged

Detector channels #1252

merged 24 commits into from
Dec 20, 2023

Conversation

phyy-nx
Copy link
Contributor

@phyy-nx phyy-nx commented Apr 6, 2023

This PR adds NXdetector_channel as a base class and adds it as optional for NXmx. @soph-dec and @kalcutter I put in placeholder documentation. Can you review and comment?

See original Dectris proposal:
https://github.com/dectris/documentation/tree/main/filewriter_v2#user-content-multi-channel-data

Closes #940
Requires #1246 to be merged first

Co-authored-by: Sophie Hotz sophie.hotz@dectris.com

Which slice of data to show in a plot by default. This is useful especially for datasets with more than 2 dimensions.
@soph-dec
Copy link
Contributor

soph-dec commented Apr 7, 2023

Thank you for the PR Aaron! First glance is looking good, I will review everything in detail next week.

Copy link
Contributor

@soph-dec soph-dec left a comment

Choose a reason for hiding this comment

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

I think we should mention NXdetector_channel in NXdetector as well. I guess something like this might do?

  <group name="CHANNELNAME_channel" type="NXdetector_channel">
      <doc>
          Group containing the description and metadata for a single channel from a multi-channel
          detector.

          Given an :ref:`NXdata` group linked as part of an NXdetector group that has an axis with
          named channels (see the example in :ref:`NXdata &lt;/NXdata@default_slice-attribute&gt;`),
          the NXdetector will have a series of NXdetector_channel groups, one for each channel,
          named CHANNELNAME_channel.
      </doc>
  </group>

base_classes/NXdetector_channel.nxdl.xml Outdated Show resolved Hide resolved
base_classes/NXdetector_channel.nxdl.xml Outdated Show resolved Hide resolved
base_classes/NXdetector_channel.nxdl.xml Outdated Show resolved Hide resolved
base_classes/NXdetector_channel.nxdl.xml Show resolved Hide resolved
phyy-nx and others added 2 commits April 18, 2023 14:32
Co-authored-by: Sophie Hotz sophie.hotz@dectris.com
Co-authored-by: soph-dec <73243774+soph-dec@users.noreply.github.com>
Also add it as optional for NXmx

See original Dectris proposal:
https://github.com/dectris/documentation/tree/main/filewriter_v2#user-content-multi-channel-data

Closes #940

Co-authored-by: Sophie Hotz <sophie.hotz@dectris.com>
phyy-nx and others added 2 commits April 18, 2023 15:02
Co-authored-by: soph-dec <73243774+soph-dec@users.noreply.github.com>
Co-authored-by: Sophie Hotz <sophie.hotz@dectris.com>
@phyy-nx
Copy link
Contributor Author

phyy-nx commented Apr 18, 2023

I think we should mention NXdetector_channel in NXdetector as well. I guess something like this might do?

  <group name="CHANNELNAME_channel" type="NXdetector_channel">
      <doc>
          Group containing the description and metadata for a single channel from a multi-channel
          detector.

          Given an :ref:`NXdata` group linked as part of an NXdetector group that has an axis with
          named channels (see the example in :ref:`NXdata &lt;/NXdata@default_slice-attribute&gt;`),
          the NXdetector will have a series of NXdetector_channel groups, one for each channel,
          named CHANNELNAME_channel.
      </doc>
  </group>

Done! Also added to NXmx

@phyy-nx phyy-nx self-assigned this Jun 19, 2023
@phyy-nx phyy-nx marked this pull request as ready for review June 19, 2023 17:19
Copy link
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.

Worth doing

phyy-nx and others added 5 commits June 20, 2023 06:04
Co-authored-by: soph-dec <73243774+soph-dec@users.noreply.github.com>
Co-authored-by: Sophie Hotz <sophie.hotz@dectris.com>

Co-authored-by: soph-dec <73243774+soph-dec@users.noreply.github.com>
Fix conflicts, adjusting phrase "Most dimensions scales will be sequences of numbers" to "Most AXISNAME fields will be sequences of numbers"
@phyy-nx
Copy link
Contributor Author

phyy-nx commented Nov 21, 2023

Conflicts fixed, no changes to this branch.

Reviewers might find it easier to use this link to compare the detector_channel branch to the dimension_as_char branch until #1246 is merged:
dimension_as_char...detector_channel

@phyy-nx phyy-nx added this to the NXDL 2023.10 milestone Nov 21, 2023
@phyy-nx phyy-nx removed the milestone label Nov 21, 2023
soph-dec added a commit to soph-dec/documentation that referenced this pull request Dec 1, 2023
@phyy-nx
Copy link
Contributor Author

phyy-nx commented Dec 6, 2023

This PR is ready for a NIAC vote. Please vote with an emoji on this comment. Options include thumbs up for yes, thumbs down for no, and anything else to abstain. Thank you. Voting will close in two weeks.

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Dec 20, 2023

Vote has passed. Thanks all.

@phyy-nx phyy-nx merged commit 5f3b4fe into main Dec 20, 2023
4 checks passed
@phyy-nx phyy-nx deleted the detector_channel branch December 20, 2023 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for Review
Development

Successfully merging this pull request may close these issues.

NXmx: Definitions for multi-channel (thresholds) data
3 participants