Skip to content
This repository has been archived by the owner on Dec 20, 2024. It is now read-only.

ENH: Implement DWI equality explicitly #161

Merged
merged 2 commits into from
May 20, 2024

Conversation

jhlegarreta
Copy link
Collaborator

@jhlegarreta jhlegarreta commented Apr 12, 2024

  • BUG: Do not pass the nibabel instance variable to get_fdata
  • ENH: Implement DWI equality explicitly

Resolves #95.

oesteban
oesteban previously approved these changes Apr 15, 2024
@jhlegarreta
Copy link
Collaborator Author

"dwi1.h5" does not exist AFAIK (or at least I put the name just to be different from "dwi.h5", and thus the PR was opened as a draft. Is there a DWI file that is not "dwi.h5" among the testing data that is pulled?

@esavary esavary requested a review from effigies May 15, 2024 19:34
@jhlegarreta
Copy link
Collaborator Author

@oesteban After having shared some more data (thanks) this can be finished. Now do we stick to the h5 file format for now for everything? I am not sure whether everything can be stored in there.

@jhlegarreta jhlegarreta force-pushed the OverloadDWIEquality branch from 34c960b to 8ce53d6 Compare May 19, 2024 01:07
@jhlegarreta jhlegarreta changed the title ENH: Overload DWI equality ENH: Implement DWI equality explicitly May 19, 2024
Do not pass the nibabel instance variable to the `get_fdata` method:
call it directly as it is a member of it.

Fixes:
```
  File "eddymotion/src/eddymotion/data/dmri.py", line 254, in load
    retval.fieldmap = fmapimg.get_fdata(fmapimg, dtype="float32")  #fmapimg.get_fdata(dtype="float32")
  File "nibabel/dataobj_images.py", line 362, in get_fdata
    raise ValueError('caching value should be "fill" or "unchanged"')
ValueError: caching value should be "fill" or "unchanged"
```
@jhlegarreta jhlegarreta force-pushed the OverloadDWIEquality branch from 8ce53d6 to b2d71a4 Compare May 19, 2024 01:22
@jhlegarreta jhlegarreta marked this pull request as ready for review May 19, 2024 01:35
@jhlegarreta jhlegarreta requested a review from effigies May 19, 2024 01:35
src/eddymotion/data/dmri.py Outdated Show resolved Hide resolved
src/eddymotion/data/dmri.py Outdated Show resolved Hide resolved
test/test_dmri.py Outdated Show resolved Hide resolved
test/test_dmri.py Outdated Show resolved Hide resolved
@jhlegarreta jhlegarreta force-pushed the OverloadDWIEquality branch 2 times, most recently from a97f879 to 2945885 Compare May 19, 2024 13:44
oesteban
oesteban previously approved these changes May 20, 2024
Copy link
Member

@oesteban oesteban left a comment

Choose a reason for hiding this comment

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

Minor nit pick

src/eddymotion/data/dmri.py Outdated Show resolved Hide resolved
Implement DWI equality explicitly: account for the cases where the
properties can be `None` by creating a helper method and exclude the
`_filepath` property as it is created using a randomly generated
dirname.
@effigies effigies merged commit 356431a into nipreps:main May 20, 2024
6 checks passed
@jhlegarreta jhlegarreta deleted the OverloadDWIEquality branch May 20, 2024 14:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DWI objects equality is broken
3 participants