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

NXmx: handle multidimensional arrays #612

Merged
merged 10 commits into from
Aug 11, 2023
Merged

NXmx: handle multidimensional arrays #612

merged 10 commits into from
Aug 11, 2023

Conversation

phyy-nx
Copy link
Contributor

@phyy-nx phyy-nx commented Mar 1, 2023

Data in NeXus can be 3 or 4 dimensional:
3D: Nimages by slow by fast
4D: Nimages by Nmodules by slow fast

Slice image_size and reshape the raw_data in these cases

Adapted from FormatNexus.py:
https://github.com/cctbx/dxtbx/blob/main/src/dxtbx/format/nexus.py#L852-L857
https://github.com/cctbx/dxtbx/blob/main/src/dxtbx/format/nexus.py#L1329-L1331

Data in NeXus can be 3 or 4 dimensional.
3D: Nimages by slow by fast
4D: Nimages by Nmodules by slow fast

Slice image_size and reshape the raw_data in these cases
@phyy-nx phyy-nx requested a review from rjgildea March 1, 2023 00:49
@codecov
Copy link

codecov bot commented Mar 1, 2023

Codecov Report

Merging #612 (969c87d) into main (d516f20) will increase coverage by 0.01%.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #612      +/-   ##
==========================================
+ Coverage   39.11%   39.13%   +0.01%     
==========================================
  Files         179      179              
  Lines       15823    15828       +5     
  Branches     3057     3057              
==========================================
+ Hits         6189     6194       +5     
  Misses       9052     9052              
  Partials      582      582              

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Mar 1, 2023

I think this line needs to be worked on as well, but I don't have a use case for it right now:
https://github.com/cctbx/dxtbx/blob/main/src/dxtbx/nexus/__init__.py#L478

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Mar 3, 2023

I think this line needs to be worked on as well, but I don't have a use case for it right now: https://github.com/cctbx/dxtbx/blob/main/src/dxtbx/nexus/__init__.py#L478

Fixed! I have a usecase for a 3D mask now

@phyy-nx phyy-nx mentioned this pull request Mar 25, 2023
@ndevenish
Copy link
Collaborator

Do you have any test files/files we can make in the right shape, to add to the testing?

@rjgildea
Copy link

rjgildea commented May 4, 2023

@phyy-nx I've attempted to add a basic test for what I understand of this use case. Could you take a look and verify that it makes sense from your perspective?

Copy link

@rjgildea rjgildea left a comment

Choose a reason for hiding this comment

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

This adds support for a use case that wasn't previously handled, LGTM 👍

src/dxtbx/nexus/__init__.py Outdated Show resolved Hide resolved
@phyy-nx
Copy link
Contributor Author

phyy-nx commented Aug 3, 2023

Hi @ndevenish I reviewed @rjgildea's test and it looks perfect. I think this is ok to merge as is.

@ndevenish ndevenish enabled auto-merge (squash) August 11, 2023 08:49
@ndevenish ndevenish merged commit 3bc6e67 into main Aug 11, 2023
17 of 20 checks passed
@ndevenish ndevenish deleted the nxmx_moredim branch August 11, 2023 09:05
toastisme pushed a commit to toastisme/dxtbx that referenced this pull request Jul 18, 2024
Data in NeXus can be 3 or 4 dimensional.
3D: Nimages by slow by fast
4D: Nimages by Nmodules by slow fast

Slice image_size and reshape the raw_data in these cases.

Co-authored-by: Richard Gildea <[email protected]>
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