ANTsX / ANTsPy

A fast medical imaging analysis library in Python with algorithms for registration, segmentation, and more.
https://antspyx.readthedocs.io
Apache License 2.0
586 stars 161 forks source link

FIX: cast unsupported numpy dtypes in ants.from_numpy() #665

Closed ncullen93 closed 1 month ago

ncullen93 commented 1 month ago

If you try to create an ants image from an array with an unsupported numpy dtype, it just fails. Since we only support 4 dtypes, we really should support automatically casting unsupported dtypes to supported ones. This PR does that.

The one question I have is whether it is good to warn the user that their data is being casted or to just do it silently. Having a bunch of warnings is annoying and pollutes the console, but I can see some people being confused that their data is in a different dtype otherwise.

Below is an example that fails. The int16 type is quite common for segmentation arrays which is how I discovered the error:

import numpy as np
import ants

arr = np.random.randn(100,100).astype('int16')
img = ants.from_numpy(arr)
coveralls commented 1 month ago

Coverage Status

coverage: 84.696% (+0.02%) from 84.673% when pulling 2043781d2f30d8e226e058170db4f7ea2a4097a4 on cast-unsupported-numpy-dtypes into e667c280e34bcb40bc6d1ab5fae372ced9eee8f5 on master.

ntustison commented 1 month ago

I'm fine either way although, correct me if I'm wrong, but silent conversion is the norm in core ANTs.

cookpa commented 1 month ago

There's also a lot of internal cloning / casting to interact with base ANTs. So even if you warn about the conversion from int16 to uint32, it's going to get cloned to float if you call apply_transforms, for example.

So I would lean towards not warning. But maybe we should allow users to set the pixel type? Clone lets you do this, but from_numpy does not.

cookpa commented 1 month ago

Might be nice to support conversion from numpy bool to uint8 as well, common masking pipeline

ncullen93 commented 1 month ago

Ok, i think im convinced to not warn. And I will include bool because that's definitely used. Thanks!