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: Remove unsafe cast during TransformBase.apply() #189

Merged
merged 1 commit into from
Mar 4, 2024

Conversation

effigies
Copy link
Member

np.asanyarray(img.dataobj, img.get_data_dtype()) is almost never a great choice. np.asanyarray(img.dataobj) is intended to be conservative with its types, where either the original dtype is maintained or scaling is applied and the dtype becomes float64. The case where this fails in particular is when the on-disk dtype is int16 with scale factors. If the scale factors send the data beyond the int16 range (slope > 1), then casting to int16 is going to truncate the data outside the range.

The approach here is to be pragmatic. We always use np.asanyarray(img.dataobj) to give the best estimate of the values in the data array that NIfTI can provide. If a dtype is provided, then we set both the output array and the image type to that, to avoid unnecessary copies and casting. If it is not provided, then the output array keeps the dtype of the input data array (the "effective dtype") and the output image keeps the on-disk dtype of the input image.

get_data_dtype() scl_slope/scl_inter output_dtype Array dtype out_img.get_data_dtype()
int16 1/0 None int16 int16
int16 2/1 None float64 int16
int16 1/0 float32 float32 float32
int16 2/1 float32 float32 float32
float32 1/0 None float32 float32
float32 2/1 None float64 float32
float32 1/0 float32 float32 float32
float32 2/1 float32 float32 float32

@effigies
Copy link
Member Author

xref nipreps/sdcflows#415

@oesteban oesteban merged commit 72058d2 into nipy:master Mar 4, 2024
6 checks passed
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.

2 participants