Skip to content

Error if EchoNumber tag missing with multi-echo sequence #347

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
1 task done
nicholst opened this issue May 16, 2019 · 2 comments · Fixed by #366
Closed
1 task done

Error if EchoNumber tag missing with multi-echo sequence #347

nicholst opened this issue May 16, 2019 · 2 comments · Fixed by #366
Labels

Comments

@nicholst
Copy link
Contributor

Summary

While PR #345 successfully treats PD+T2 pairs as multi-echo, we now get runtime errors when such acquisitions are missing the EchoNumber tag:

*Runtime**Error**: was asked to copy X but destination already exists: Y

Using the --overwrite option eliminates the error but only leaves us with one echo.

Presently, multi echo support depends on EchoNumber tag, but given there are DICOM files in the wild that lack this tag we need a backup logic, presumably based on EchoTime. I can see that this requires a modification to the logic of save_converted_files in convert.py... something like

EchoNumberBackup = [x+1 for x in sorted(range(len(echo_times)), key=echo_times.__getitem__)]

and then assignment of echo_number will need to check for empty value and use EchoNumberBackup when missing. I can almost see what to do but it's just a bit beyond my very elementary Python.

Platform details:

Choose one:

  • Local environment

Python 3.6.6

  • Heudiconv version:

v0.5.4 + PR #345 patch

@yarikoptic
Copy link
Member

another related is #346 - "Provide flexible support for multi-volume sequences" ;-)

@mgxd mgxd added the bug label May 16, 2019
@tsalo
Copy link
Member

tsalo commented Aug 7, 2019

I fixed this on my fork by converting echo_times into a list and using the file's EchoTime to index that list (plus one) to get the echo number if EchoNumber wasn't in fileinfo's keys.

I.e.,

# Get the EchoNumber from json file info.  If not present, use EchoTime
if 'EchoNumber' in fileinfo.keys():
    echo_number = fileinfo['EchoNumber']
else:
    echo_number = echo_times.index(fileinfo['EchoTime']) + 1

I'd be happy to open a PR, unless there are plans for a different fix.

@mgxd mgxd closed this as completed in #366 Aug 21, 2019
yarikoptic added a commit that referenced this issue Dec 16, 2019
This is largely a bug fix.  Metadata and order of `_key-value` fields in BIDS
could change from the result of converting using previous versions, thus minor
version boost.
14 people contributed to this release -- thanks
[everyone](https://github.com/nipy/heudiconv/graphs/contributors)!

 Enhancement

- Use [etelemetry](https://pypi.org/project/etelemetry) to inform about most
  recent available version of heudiconv. Please set `NO_ET` environment variable
  if you want to disable it ([#369][])
- BIDS:
  - `--bids` flag became an option. It can (optionally) accept `notop` value
    to avoid creation of top level files (`CHANGES`, `dataset_description.json`,
    etc) as a workaround during parallel execution to avoid race conditions etc.
    ([#344][])
  - Generate basic `.json` files with descriptions of the fields for
    `participants.tsv` and `_scans.tsv` files ([#376][])
  - Use `filelock` while writing top level files. Use
    `HEUDICONV_FILELOCK_TIMEOUT` environment to change the default timeout value
    ([#348][])
  - `_PDT2` was added as a suffix for multi-echo (really "multi-modal")
    sequences ([#345][])
- Calls to `dcm2niix` would include full output path to make it easier to
  discern in the logs what file it is working on ([#351][])
- With recent [datalad]() (>= 0.10), created DataLad dataset will use
  `--fake-dates` functionality of DataLad to not leak data conversion dates,
  which might be close to actual data acquisition/patient visit ([#352][])
- Support multi-echo EPI `_phase` data ([#373][] fixes [#368][])
- Log location of a bad .json file to ease troubleshooting ([#379][])
- Add basic pypi classifiers for the package ([#380][])

 Fixed
- Sorting `_scans.tsv` files lacking valid dates field should not cause a crash
  ([#337][])
- Multi-echo files detection based number of echos ([#339][])
- BIDS
  - Use `EchoTimes` from the associated multi-echo files if `EchoNumber` tag is
    missing ([#366][] fixes [#347][])
  - Tolerate empty ContentTime and/or ContentDate in DICOMs ([#372][]) and place
    "n/a" if value is missing ([#390][])
  - Do not crash and store original .json file is "JSON pretification" fails
    ([#342][])
- ReproIn heuristic
  - tollerate WIP prefix on Philips scanners ([#343][])
  - allow for use of `(...)` instead of `{...}` since `{}` are not allowed
    ([#343][])
  - Support pipolar fieldmaps by providing them with `_epi` not `_magnitude`.
    "Loose" BIDS `_key-value` pairs might come now after `_dir-` even if they
    came first before ([#358][] fixes [#357][])
- All heuristics saved under `.heudiconv/` under `heuristic.py` name, to avoid
  discrepancy during reconversion ([#354][] fixes [#353][])
- Do not crash (with TypeError) while trying to sort absent file list ([#360][])
- heudiconv requires nipype >= 1.0.0 ([#364][]) and blacklists `1.2.[12]` ([#375][])

* tag 'v0.6.0': (60 commits)
  Version boost to 0.6.0
  DOC: populate detailed changelog for 0.6.0 and tune up formatting in previous one
  Fix miscellaneous typos in ReproIn heuristic file.
  BF: fix check for the sbatch (SLURM) not being available
  ENH: make test-compare-two-versions take any two worktrees, and just show diff if results already known
  Update heudiconv/convert.py
  apply @mgxd 's suggestions, adding a warning and a timeout environment variable
  need str typecast
  Use empty string not None
  Empty acq_time results in empty cell not 'n/a'
  DOC: Clarify tarball session handling
  remove repetitive import statement
  respond to review - add explicit py2 check - change file saving strategy - use logger instead of print
  fix remaning py2 errors
  MNT: Add Python support metadata to package
  fix some python2/3 incompatibilities
  add return data (accidently removed return)
  make content unicode
  test that load_json provides filename if invalid
  explicitly name invalid json
  ...
yarikoptic added a commit that referenced this issue Dec 16, 2019
[0.6.0] - 2019-12-16

This is largely a bug fix.  Metadata and order of `_key-value` fields in BIDS
could change from the result of converting using previous versions, thus minor
version boost.
14 people contributed to this release -- thanks
[everyone](https://github.com/nipy/heudiconv/graphs/contributors)!

Enhancement

- Use [etelemetry](https://pypi.org/project/etelemetry) to inform about most
  recent available version of heudiconv. Please set `NO_ET` environment variable
  if you want to disable it ([#369][])
- BIDS:
  - `--bids` flag became an option. It can (optionally) accept `notop` value
    to avoid creation of top level files (`CHANGES`, `dataset_description.json`,
    etc) as a workaround during parallel execution to avoid race conditions etc.
    ([#344][])
  - Generate basic `.json` files with descriptions of the fields for
    `participants.tsv` and `_scans.tsv` files ([#376][])
  - Use `filelock` while writing top level files. Use
    `HEUDICONV_FILELOCK_TIMEOUT` environment to change the default timeout value
    ([#348][])
  - `_PDT2` was added as a suffix for multi-echo (really "multi-modal")
    sequences ([#345][])
- Calls to `dcm2niix` would include full output path to make it easier to
  discern in the logs what file it is working on ([#351][])
- With recent [datalad]() (>= 0.10), created DataLad dataset will use
  `--fake-dates` functionality of DataLad to not leak data conversion dates,
  which might be close to actual data acquisition/patient visit ([#352][])
- Support multi-echo EPI `_phase` data ([#373][] fixes [#368][])
- Log location of a bad .json file to ease troubleshooting ([#379][])
- Add basic pypi classifiers for the package ([#380][])

Fixed

- Sorting `_scans.tsv` files lacking valid dates field should not cause a crash
  ([#337][])
- Multi-echo files detection based number of echos ([#339][])
- BIDS
  - Use `EchoTimes` from the associated multi-echo files if `EchoNumber` tag is
    missing ([#366][] fixes [#347][])
  - Tolerate empty ContentTime and/or ContentDate in DICOMs ([#372][]) and place
    "n/a" if value is missing ([#390][])
  - Do not crash and store original .json file is "JSON pretification" fails
    ([#342][])
- ReproIn heuristic
  - tolerate WIP prefix on Philips scanners ([#343][])
  - allow for use of `(...)` instead of `{...}` since `{}` are not allowed
    ([#343][])
  - Support pipolar fieldmaps by providing them with `_epi` not `_magnitude`.
    "Loose" BIDS `_key-value` pairs might come now after `_dir-` even if they
    came first before ([#358][] fixes [#357][])
- All heuristics saved under `.heudiconv/` under `heuristic.py` name, to avoid
  discrepancy during reconversion ([#354][] fixes [#353][])
- Do not crash (with TypeError) while trying to sort absent file list ([#360][])
- heudiconv requires nipype >= 1.0.0 ([#364][]) and blacklists `1.2.[12]` ([#375][])

* tag 'v0.6.0':
  Boost perspective release date in changelog to today
  ENH(TST): Fix version to older pytest to ease backward compatibility testing
  RF: use tmpdir not tmp_path fixture
  FIX: minor typo in CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants