Skip to content

Intent to deprecate: 64-bit integer NIfTI images #1089

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

Closed
effigies opened this issue Feb 18, 2022 · 10 comments
Closed

Intent to deprecate: 64-bit integer NIfTI images #1089

effigies opened this issue Feb 18, 2022 · 10 comments

Comments

@effigies
Copy link
Member

effigies commented Feb 18, 2022

64 bit integer images are easy to generate in nibabel due to numpy semantics, but almost universally incompatible with non-Python tools. We must balance breaking existing Python scripts, the Python library ecosystem, and the wider neuroimaging ecosystem that uses NIfTI as an interchange format.

Recognizing that we are currently contributing to breakage in the wider ecosystem, we have decided to deprecate 64-bit integer image creation except via explicit override. The following path is proposed in order to bring major Python libraries into alignment over a 6-month window through escalating warnings and errors.

NiBabel will take the following steps:

cc @matthew-brett @jbteves @neurolabusc @mrneont @jeromedockes @NicolasGensollen

Please add anything I've missed or let me know if anything needs adjustment.

@effigies effigies pinned this issue Feb 18, 2022
@jbteves
Copy link

jbteves commented Feb 18, 2022

Thanks for writing this up! I would add to the dtype parameter that you should explicitly test to make sure that if somebody passes the int alias, it errors out as invalid so that we avoid the int32/int64 ambiguity when discussing "nifti" int versus the "numpy" int. I'm not sure exactly how the type resolution might work so I wanted to make sure that was down.

@neurolabusc
Copy link

@effigies thanks for the concise summary.

I like @jbteves idea, but I doubt there is a way to distinguish an explicit np.int64 from the common way people define int

>>> A = np.array([1,123,0], np.int64)
>>> A.dtype
dtype('int64')
>>> B = np.array([1,123,0], int)
>>> B.dtype
dtype('int64')

@jbteves
Copy link

jbteves commented Feb 18, 2022

I know this is hacky but you can actually use the __str__ method to disambiguate:

>>> ambiguous = int
>>> explicit = np.int64
>>> print(str(ambiguous))
<class 'int'>
>>> print(str(explicit))
<class 'numpy.int64'>

So you can do something like this:

def check_explicit(x):
    return str(x) != "<class 'int'>"
print(check_explicit(int))
print(check_explicit(np.int64))

which results in

False
True

@jbteves
Copy link

jbteves commented Feb 18, 2022

Note that the Python int type will actually expand to meet its precision needs, while np.int64 will overflow. So while there are some rules that make this complicated, they are ultimately different types. Most pertinently, we can force the user to be explicit in this case.

@neurolabusc
Copy link

@jbteves wow! I did not know that. That is a very nice trick. It does provide a future mechanism for a user to explicitly demand int64 if they really want it.

@matthew-brett
Copy link
Member

@jbteves - ah - but it's subtle, because dtype=int, as interpreted by Numpy, doesn't give the infinite size Python int, but gives one of np.int64 or np.int32, depending on your platform. So because:

[ins] In [3]: np.zeros(4, dtype=int).dtype
Out[3]: dtype('int64')

the user might expect the same for dtype=int when saving in Nibabel.

@jbteves
Copy link

jbteves commented Feb 18, 2022

Yes, I agree! I more meant for the purposes of this particular saving function, if we want a user to be very explicit we can use the string casting to check if they used int alias or provided a fixed precision like np.int64 for us to use, avoiding the ambiguity referenced on our Zoom call. Unless you're aware of a way this could be circumvented? I had assumed the function would look like this:

def to_file(self, fname, dtype=None, ...):
    # do stuff
    if not dtype:
          # use the header
    else:
         if isinstance(dtype, str): # do stuff, complain about 'int'
         elif isinstance(dtype, type):
              if str(dtype) == "<class 'int'>" raise ValueError("Please provide a fixed precision")
              # we should be golden now for integers, maybe we want to force the same for floats??

@matthew-brett
Copy link
Member

Yes - sure - or maybe

if issubclass(dtype, int):
    raise ValueError("Please ... etc")

?

@jbteves
Copy link

jbteves commented Feb 18, 2022

That's much less hacky, good idea! I always forget about Python's inheritance features.

@effigies
Copy link
Member Author

effigies commented Feb 18, 2022

Just to make it concrete, I was thinking something quite simple: 46bf959. (This is on the __init__ side. Would do a similar thing on the save side.

@effigies effigies unpinned this issue Jun 6, 2022
davidt0x added a commit to davidt0x/brainiak that referenced this issue May 16, 2023
See nipy/nibabel#1089

np.array using int64 for arrays created with all integers. These
arrays cannot be passed to directly to Nifti1Pair constructor
anymore without explicit dtype argument specifying that the user
wants a int64 image. I added a dtype=float specification to the
np.array construction to get rid of this error.
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

No branches or pull requests

4 participants