BF: fix repetition time in PARREC header

[Roosted7]

While I was working on #683, I noticed that the scaling in the repetition time header field is wrong.
It looks like the last value of get_zooms() should report the repetition time in milliseconds and not in seconds.

Ofcourse, I also fixed the tests.

[coveralls]

Coverage remained the same at 91.851% when pulling f29154a on Roosted7:parrec_fix_Tr into ad26878 on nipy:master.

[effigies]

Please merge master to fix style checks.

[codecov-io]

Codecov Report

Merging #692 into master will decrease coverage by 0.66%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #692      +/-   ##
==========================================
- Coverage   89.55%   88.88%   -0.67%     
==========================================
  Files          93       93              
  Lines       11474    11449      -25     
  Branches     1992     1892     -100     
==========================================
- Hits        10275    10176      -99     
- Misses        859      933      +74     
  Partials      340      340

Impacted Files	Coverage Δ
nibabel/parrec.py	94.39% <100%> (ø)	⬆️
nibabel/py3k.py	35.71% <0%> (-37.51%)	⬇️
nibabel/pydicom_compat.py	65% <0%> (-35%)	⬇️
nibabel/pkg_info.py	58.62% <0%> (-20.69%)	⬇️
nibabel/environment.py	75% <0%> (-20%)	⬇️
nibabel/nicom/dicomreaders.py	42.85% <0%> (-16.33%)	⬇️
nibabel/openers.py	76.47% <0%> (-4.99%)	⬇️
nibabel/casting.py	84.74% <0%> (-2.55%)	⬇️
nibabel/filename_parser.py	91.46% <0%> (-2.44%)	⬇️
nibabel/testing/__init__.py	95.83% <0%> (-0.98%)	⬇️
... and 13 more

---

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 9430224...f29154a. Read the comment docs.

[Roosted7]

@effigies Sure! I rebased my pr on master

[effigies]

Thanks. Just out of curiosity, why use milliseconds over seconds? Simply to transparently report the values in the file, or was there some other bug? I ask because a lot of code assumes that get_zooms()[3] is in seconds, regardless of image type.

[Roosted7]

My attention was drawn to this after hearing the reason why my colleagues are using dcm2niix over Nibabel's parrec2nii; the BIDS tools they use to read Nifti's (like fmriprep) expect the value to be in milliseconds and do not accept the files produced by parrec2nii.

Also, it seems a lot cleaner to transparently report the value in the value.

[effigies]

fMRIPrep was not written to expect TRs to be in milliseconds, and in fact ignores the NIfTI file TRs altogether, preferring the RepetitionTime metadata in the JSON sidecars (which is required by BIDS to be in seconds, FWIW).

Is it possible that nibabel is outputting NIfTIs with TRs in seconds but reporting millisecond units? That could cause problems.

[Roosted7]

JSON sidecarts are not required for fMRIPrep to run right?

That would explain these problems indeed, but is there a field in the NifTI header that sets the units for the Tr?

[effigies]

fMRIPrep assumes a valid BIDS dataset. While not all mandatory metadata is accessed, we do attempt to access some of it, including RepetitionTime, so if it runs, you must have that.

The xyzt_units field of the NIfTI header can includes the bitwise AND of spatial and temporal units, with codes described here:

nibabel/nibabel/nifti1.py

Lines 130 to 141 in ad26878

	# unit codes
	unit_codes = Recoder(( # code, label
	(0, 'unknown'),
	(1, 'meter'),
	(2, 'mm'),
	(3, 'micron'),
	(8, 'sec'),
	(16, 'msec'),
	(24, 'usec'),
	(32, 'hz'),
	(40, 'ppm'),
	(48, 'rads')), fields=('code', 'label'))

A code of 10 indicates spatial units of mm and temporal units of sec, and most software will assume those if either code is missing, and many programs will assume those regardless of the actual units specified (I recall seeing a peripheral FSL program that did not check units and got tripped up by ms TR within the last two years).

To quickly view the shape and units of a file you have (if nibabel is installed):

nib-ls -H dim,pixdim,xyzt_units file.nii.gz

[effigies]

Hi @Roosted7, just checking in on what your thoughts are on this? Are you still having issues with some software expecting milliseconds? Could you provide a test where a file is written with bad metadata?

[epongpipat]

I'm not sure if this is fixed yet, but I'm also having this issue. The TR number is being converted to sec, but the time scale is still in msec. When I run nib-ls -H xyzt_units I get the 18 rather than 10 for the xyzt_units.

[effigies]

@epongpipat Sorry about the extreme delay, but thanks for your comment. I had a poke around parrec2nii, and found that what seems to be happening is that the zooms are in seconds, but there is a method to enable NIfTI conversion here:

nibabel/nibabel/parrec.py

Lines 766 to 780 in 7a7a913

	def as_analyze_map(self):
	"""Convert PAR parameters to NIFTI1 format"""
	# Entries in the dict correspond to the parameters found in
	# the NIfTI1 header, specifically in nifti1.py `header_dtd` defs.
	# Here we set the parameters we can to simplify PAR/REC
	# to NIfTI conversion.
	descr = ("%s;%s;%s;%s"
	% (self.general_info['exam_name'],
	self.general_info['patient_name'],
	self.general_info['exam_date'].replace(' ', ''),
	self.general_info['protocol_name']))[:80] # max len
	is_fmri = (self.general_info['max_dynamics'] > 1)
	t = 'msec' if is_fmri else 'unknown'
	xyzt_units = unit_codes['mm'] + unit_codes[t]
	return dict(descr=descr, xyzt_units=xyzt_units) # , pixdim=pixdim)

This reports msec instead of sec. I proposed an alternative fix in #931.

[effigies]

Closing for now.