NF: Pass dtype to Analyze-like images at initialization/serialization, warn on creation of NIfTI images with 64-bit ints (API change)

[effigies]

No test yet, but a quick prototype based on the discussion in #1046.

I'm not sure what people's intuition about the behavior of a dtype parameter should be. What I've done for now is just to set_data_dtype() so that the array is untouched but will be coerced at write time. However, I could see an argument that the expectation is that the array would be immediately cast so that the result of np.asanyarray(img.dataobj) will have the dtype passed to the image.

Additionally, this currently assumes that passing a header is as good as passing a dtype, but this is going to have the problem that passing the header of a 64-bit image will succeed silently.

I have not made any attempt to implement a from nibabel.__future__ import int64_error at this point. Figured I'd get some code out and argue out the semantics before refining.

[codecov]

Codecov Report

Merging #1082 (fa66516) into master (7cfaebf) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1082      +/-   ##
==========================================
+ Coverage   92.29%   92.30%   +0.01%     
==========================================
  Files         100      100              
  Lines       12247    12269      +22     
  Branches     2393     2399       +6     
==========================================
+ Hits        11303    11325      +22     
  Misses        621      621              
  Partials      323      323              

Impacted Files	Coverage Δ
nibabel/funcs.py	80.28% <ø> (ø)
nibabel/analyze.py	98.55% <100.00%> (+0.02%)	⬆️
nibabel/arrayproxy.py	100.00% <100.00%> (ø)
nibabel/cifti2/cifti2.py	96.65% <100.00%> (ø)
nibabel/filebasedimages.py	89.36% <100.00%> (ø)
nibabel/loadsave.py	93.84% <100.00%> (ø)
nibabel/nifti1.py	92.68% <100.00%> (+0.09%)	⬆️
nibabel/spm99analyze.py	91.04% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7cfaebf...fa66516. Read the comment docs.

[matthew-brett]

I think the logic is right - that the dtype parameter overrides whatever is in the header, if present. I am personally OK with not warning about int64 in the header - if the header got to here, the user must be OK with int64 images. After this change, they will be difficult to make by accident.

[matthew-brett]

Thinking further about the future import - I think we'll have to make that a module global in fact, as in:

# Using it
nib.__future__.int64_error = True

[pep8speaks]

Hello @effigies, Thank you for updating!

Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, pip install flake8 and then run flake8 nibabel.

Comment last updated at 2022-03-29 14:43:35 UTC

[effigies]

Dropped the __future__ config, since we're just going to do a rapid warn/error cycle.

[effigies]

This is ready for review. This PR aims to establish the API and semantics for explicit dtypes. I have not complicated this PR by trying to add in keywords like "compat" or "mask"; I think that will be cleaner as a separate PR that builds on what we end up with here.

@matthew-brett @jbteves @jeromedockes

[jeromedockes]

I don't know nibabel well enough to do an actual review but this seems to be in line with what was decided during the nilearn office hours.

[effigies]

@jeromedockes Sorry, yes, in pinging you, I mostly meant "Does this API work for nilearn's purposes?"

[jeromedockes]

No problem, on the contrary thanks for pinging me! yes I do think that API will work for nilearn. We haven't really made progress because the nilearn developers team is at a somewhat reduced capacity at the moment but we should get to it soon. if you do open the follow-up PR adding some keywords please ping us there too as we might take advantage of these as well.

[effigies]

@matthew-brett Could I bug you for a review?

[effigies]

@matthew-brett Any chance of a review? I think this one should be relatively straightforward while #1082 will take more careful thought.

[effigies]

Going to merge. Can fix up later if something is found. Realizing we didn't get CIFTI fully supported on the creation side, but that's turning out to be complicated enough to be worth its own PR.