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

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

Merged
merged 3 commits into from
May 27, 2024

Conversation

ncullen93
Copy link
Member

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
Copy link

coveralls commented May 26, 2024

Coverage Status

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

@ntustison
Copy link
Member

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

@cookpa
Copy link
Member

cookpa commented May 26, 2024

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
Copy link
Member

cookpa commented May 26, 2024

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

@ncullen93
Copy link
Member Author

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

@ncullen93 ncullen93 merged commit 342603f into master May 27, 2024
2 checks passed
@ncullen93 ncullen93 deleted the cast-unsupported-numpy-dtypes branch May 27, 2024 11:50
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