Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
ENH: Add nibabel-based split and merge interfaces #489
ENH: Add nibabel-based split and merge interfaces #489
Changes from 20 commits
0de7374
bfcd29c
30ddf03
5e3c93c
40f6de1
f3572e6
49ff6bd
87f1cb3
b4fcdb7
f51f510
9aa0655
5a31b01
53db81a
1c27f20
05724d5
7263d03
03ebb6d
9446d47
0ae5351
fc42351
1d12dd3
d657546
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason that non-terminal 1-dimensions are left in place by
squeeze_image
is to preserve the meaning of each dimension. For example, if I have a time series of a single slice(64, 64, 1, 48)
,squeeze_image
will preserve the meaning ofi, j, k, n
, butnp.squeeze
will recastn
ask
.What's the use case that you're taking care of here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the use case I'm contemplating is separating out the three components of deformation fields and other model-based nonlinear transforms. There is one example of this in the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found it. I guess I would think splitting along the fifth dimension is a different task from splitting along the fourth, but I see that it's convenient to have a single interface. I'm not sure that risking dropping meaningful dimensions is a good idea.
What about forcing to 4D like:
This coerces a 3D image to (x, y, z, 1) and a 4+D image to (x, y, z, n) assuming that dimensions 4-7 are all 1, n or absent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it guaranteed that the spatial dimensions will not be affected on that reshape? If so, I'm down with this solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trivial dimensions have no effect on indexing, whether it's C- or Fortran ordered. If you don't change the order, it's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is odd, as the above will coerce a valid 4D
(x, y, z, 1)
image to 3D(x, y, z)
, requiring you to thenallow_3D
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is that odd? It is indeed a 3D volume, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for instance, there are a fair number of T1w images in OpenNeuro with (x, y, z, 1) dimensions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a 4D series. You should be able to split it into one 3D volume without special casing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the above snippet you wrote would make this particular use-case a standard one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say checking for 3D volumes should happen before reshaping the image.