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

sct_deepseg_sc - Issue when input is .nii instead of .nii.gz #1706

Merged
merged 2 commits into from
Apr 30, 2018
Merged

Conversation

charleygros
Copy link
Member

Requirements

This issue was reported twice:

Description of the Change

>>> from msct_image import Image
>>> s_compressed=Image('t2_seg.nii.gz')
>>> type(s_compressed.data)
<type 'numpy.ndarray'>
>>> s_not_compressed=Image('t2_seg.nii')
>>> type(s_not_compressed.data)
<class 'numpy.core.memmap.memmap'>

>>> from scipy.ndimage.measurements import center_of_mass
>>> from msct_image import Image

>>> s_not_compressed = Image('t2_seg.nii')
>>> type(s_not_compressed.data)
<class 'numpy.core.memmap.memmap'>
>>> center_of_mass(s_not_compressed.data)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/chgroc/spinalcordtoolbox/python/lib/python2.7/site-packages/scipy/ndimage/measurements.py", line 1289, in center_of_mass
    return [tuple(v) for v in numpy.array(results).T]
TypeError: 'numpy.float64' object is not iterable

>>> s_compressed = Image('t2_seg.nii.gz')
>>> type(s_compressed.data)
<type 'numpy.ndarray'>
>>> center_of_mass(s_compressed.data)
(225.70722563053852, 216.90064758009544, 7.8552317655078392)

--> Error when scipy.ndimage.measurements.center_of_mass receives a numpy.core.memmap.memmap as input.

Proposed solution:
Cast np.memmap into np.array before to apply scipy.ndimage.measurements.center_of_mass.

Is there a better approach?


Additional tests were done on ~30 data (different contrast, centers...etc) --> works

@zougloub
Copy link
Contributor

zougloub commented Apr 29, 2018

numpy limitation (other people have seen it: https://stackoverflow.com/a/25370699 / numpy/numpy#667) and we still have it because we're bundling an archaic version of numpy.

@zougloub zougloub added the upstream Issue caused by software dependencies label Apr 29, 2018
@@ -108,7 +108,7 @@ def crop_image_around_centerline(im_in, ctr_in, im_out, crop_size, x_dim_half, y

x_lst, y_lst = [], []
for zz in range(im_in.dim[2]):
x_ctr, y_ctr = center_of_mass(im_ctr.data[:, :, zz])
x_ctr, y_ctr = center_of_mass(data_ctr[:, :, zz])
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd put the np.array() cast right in there, so it is more obvious that we're doing it for center_of_mass().

Copy link
Member Author

Choose a reason for hiding this comment

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

True, I am modifying it

@zougloub
Copy link
Contributor

You may amend your commit and re-push your branch to have the corresponding issue ids in the commit title. But it's not bad as is.

@jcohenadad jcohenadad removed their request for review April 30, 2018 01:56
@zougloub zougloub merged commit 75a7408 into master Apr 30, 2018
@zougloub zougloub deleted the cg_i1634 branch April 30, 2018 14:25
@jcohenadad jcohenadad added this to the 3.2.0 milestone May 29, 2018
@jcohenadad jcohenadad added bug category: fixes an error in the code and removed card:TO_REVIEW labels May 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug category: fixes an error in the code sct_deepseg_sc context: upstream Issue caused by software dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants