Skip to content
Merged
20 changes: 18 additions & 2 deletions nibabel/externals/netcdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,14 @@ def __init__(self, filename, mode='r', mmap=None, version=1):
self.fp = open(self.filename, '%sb' % mode)
if mmap is None:
mmap = True
try:
self.fp.seek(0, 2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the ValueError comes here? Best to isolate that line with try, as in:

try:
    self.fp.seek(0, 2)
except ValueError:
    self.file_bytes = -1  # Unknown file length (gzip).
else:
    self.file_bytes = self.fp.tell()
    self.fp.seek(0)

except ValueError:
self.file_bytes = -1 # Unknown file length (gzip).
else:
self.file_bytes = self.fp.tell()
self.fp.seek(0)

self.use_mmap = mmap
self.version_byte = version

Expand Down Expand Up @@ -599,14 +607,22 @@ def _read_var_array(self):
else: # not a record variable
# Calculate size to avoid problems with vsize (above)
a_size = reduce(mul, shape, 1) * size
if self.use_mmap:
if self.file_bytes >= 0 and begin_ + a_size > self.file_bytes:
data = fromstring(b'\x00'*a_size, dtype=dtype_)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces around the *.

elif self.use_mmap:
mm = mmap(self.fp.fileno(), begin_+a_size, access=ACCESS_READ)
data = ndarray.__new__(ndarray, shape, dtype=dtype_,
buffer=mm, offset=begin_, order=0)
else:
pos = self.fp.tell()
self.fp.seek(begin_)
data = fromstring(self.fp.read(a_size), dtype=dtype_)
# Try to read file, which may fail because the data is
# at or past the end of file. In that case, we treat
# this data as zeros.
buf = self.fp.read(a_size)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment here? I guess this is for a broken file where there's an expected variable truncated at the end? Do you really want to allow that case without raising an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this error is absolutely "normal" in the sense that I routinely see files that have this condition.

if len(buf) < a_size:
buf = b'\x00'*a_size
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces around the *.

data = fromstring(buf, dtype=dtype_)
data.shape = shape
self.fp.seek(pos)

Expand Down
16 changes: 7 additions & 9 deletions nibabel/minc1.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ def get_data_shape(self):
def get_zooms(self):
""" Get real-world sizes of voxels """
# zooms must be positive; but steps in MINC can be negative
return tuple(
[abs(float(dim.step)) for dim in self._dims])
return tuple([abs(float(dim.step)) if hasattr(dim, 'step') else 1.0
for dim in self._dims])

def get_affine(self):
nspatial = len(self._spatial_dims)
Expand All @@ -106,13 +106,11 @@ def get_affine(self):
dim_names = list(self._dim_names) # for indexing in loop
for i, name in enumerate(self._spatial_dims):
dim = self._dims[dim_names.index(name)]
try:
dir_cos = dim.direction_cosines
except AttributeError:
dir_cos = _default_dir_cos[name]
rot_mat[:, i] = dir_cos
steps[i] = dim.step
starts[i] = dim.start
rot_mat[:, i] = (dim.direction_cosines
if hasattr(dim, 'direction_cosines')
else _default_dir_cos[name])
steps[i] = dim.step if hasattr(dim, 'step') else 1.0
starts[i] = dim.start if hasattr(dim, 'start') else 0.0
origin = np.dot(rot_mat, starts)
aff = np.eye(nspatial + 1)
aff[:nspatial, :nspatial] = rot_mat * steps
Expand Down
7 changes: 5 additions & 2 deletions nibabel/minc2.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,10 @@ def _get_dimensions(self, var):
dimorder = var.attrs['dimorder'].decode()
except KeyError: # No specified dimensions
return []
return dimorder.split(',')
# The dimension name list must contain only as many entries
# as the variable has dimensions. This reduces errors when an
# unnecessary dimorder attribute is left behind.
return dimorder.split(',')[:len(var.shape)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give an example of a failure here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check in a test case. Some MINC files contain stray "dimorder" attributes that are longer than the variable itself, e.g. a file may have a scalar image-min with a dimorder=zspace attribute.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test case would be good. Could you add a comment explaining as well? Otherwise it might be puzzling for someone coming fresh to the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MINC 2 format file I added to the test cases will exercise this.


def get_data_dtype(self):
return self._image.dtype
Expand All @@ -95,7 +98,7 @@ def _get_valid_range(self):
info = np.iinfo(ddt.type)
try:
valid_range = self._image.attrs['valid_range']
except AttributeError:
except (AttributeError, KeyError):
valid_range = [info.min, info.max]
else:
if valid_range[0] < info.min or valid_range[1] > info.max:
Expand Down
Binary file added nibabel/tests/data/minc1-no-att.mnc
Binary file not shown.
Binary file added nibabel/tests/data/minc2-4d-d.mnc
Binary file not shown.
Binary file added nibabel/tests/data/minc2-no-att.mnc
Binary file not shown.
15 changes: 15 additions & 0 deletions nibabel/tests/test_minc1.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,21 @@
max=1.498039216,
mean=0.9090422837),
is_proxy=True),
dict(
fname=pjoin(data_path, 'minc1-no-att.mnc'),
shape=(10, 20, 20),
dtype=np.uint8,
affine=np.array([[0, 0, 1.0, 0],
[0, 1.0, 0, 0],
[1.0, 0, 0, 0],
[0, 0, 0, 1]]),
zooms=(1., 1., 1.),
# These values from SPM2/mincstats
data_summary=dict(
min=0.20784314,
max=0.74901961,
mean=0.6061103),
is_proxy=True),
]


Expand Down
32 changes: 31 additions & 1 deletion nibabel/tests/test_minc2.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,37 @@
min=0.2078431373,
max=1.498039216,
mean=0.9090422837),
is_proxy=True)
is_proxy=True),
dict(
fname=pjoin(data_path, 'minc2-no-att.mnc'),
shape=(10, 20, 20),
dtype=np.uint8,
affine=np.array([[0, 0, 1.0, 0],
[0, 1.0, 0, 0],
[1.0, 0, 0, 0],
[0, 0, 0, 1]]),
zooms=(1., 1., 1.),
# These values from SPM2/mincstats
data_summary=dict(
min=0.20784314,
max=0.74901961,
mean=0.6061103),
is_proxy=True),
dict(
fname=pjoin(data_path, 'minc2-4d-d.mnc'),
shape=(5, 16, 16, 16),
dtype=np.float64,
affine=np.array([[1., 0., 0., -6.96 ],
[0., 1., 0., -12.453],
[0., 0., 1., -9.48 ],
[0., 0., 0., 1.]]),
zooms=(1., 1., 1., 1.),
# These values from mincstats
data_summary=dict(
min=0.0,
max=5.0,
mean=2.00078125),
is_proxy=True),
]

if have_h5py:
Expand Down