Repository: nipy/heudiconv

Issue:
enhanced dicoms (XA) and sequence_name in PulseSequenceName dicom tag
<!-- DO NOT DELETE THIS!
This template is used to facilitate issue resolution.
All text in <!-> tags will not be displayed.
-->

### Summary
<!-- If you are having conversion troubles, please share as much
relevant information as possible. This includes, but is not limited to:
- log of conversion
- heuristic
-->

Finally got some Siemens XA dicoms in hands, and the `sequence_name` is now in `(0018,9005) PulseSequenceName` dicom tag, resulting in the seqinfo `sequence_name` to be empty.
I can submit a patch to the following, to extract this value when the other regular tags are missing.

https://github.com/nipy/heudiconv/blob/7caff37cf040d0ba12967b41b0385548ce6a51bd/heudiconv/dicoms.py#L91-L98

### Platform details:

Choose one:
- [x] Local environment
<!-- If selected, please provide OS and python version -->
- [ ] Container
<!-- If selected, please provide container name and tag"-->
ubuntu, python3.10

- Heudiconv version:
<!-- To check: run heudiconv with just the --version flag -->
1.1.0

Discussion:
yarikoptic: Hi @bpinsard -- sounds important. Any sample dicoms ?  may be some among dcm2niix test ones (ping @rordenlab)
bpinsard: The XA dicoms I got are not mine. However, it seems that the dcm2niix XA test data show that behavior. eg. https://github.com/neurolabusc/dcm_qa_nih/blob/master/Ref/DCM2NIIX_regression_test_4.json only has PulseSequenceName and not SequenceName tag.
yarikoptic: ah, what could go wrong, right? ;-)  that `sequence_name` was added by @leej3 in 2017 in 5a7fb1f869d4ee9e2a0d415551e2639d5b1b6af7 and it seems that no existing here heuristics use it:

```
❯ git grep '\[19\]' | grep -v '\.dcm'
❯ git grep sequence_name
heudiconv/dicoms.py:        sequence_name = dcminfo[0x18, 0x24].value
heudiconv/dicoms.py:        sequence_name = dcminfo[0x19, 0x109C].value
heudiconv/dicoms.py:        sequence_name = ""
heudiconv/dicoms.py:        sequence_name=sequence_name,
heudiconv/utils.py:    sequence_name: str  # 19
```

and no tests test it? (not good).  FWIW on my Siemens sample files I do not see that field at all 

> The XA dicoms I got are not mine. However, it seems that the dcm2niix XA test data show that behavior. eg. https://github.com/neurolabusc/dcm_qa_nih/blob/master/Ref/DCM2NIIX_regression_test_4.json only has PulseSequenceName and not SequenceName tag.

well -- there the epiRT seems to be found only in GE data if I get naming of folders correctly:

```
❯ pwd
/home/yoh/deb/gits/pkg-exppsy/dcm2niix/dcm_qa_nih/In/20180918Si
❯ dcmdump */*dcm | grep epiRT | head
❯ cd -
~exppsy/dcm2niix/dcm_qa_nih/In/20180918GE
README-Study.txt*  mr_0004/  mr_0005/  mr_0006/  mr_0007/
❯ dcmdump */*dcm | grep epiRT | head
(0019,109c) LO [epiRT]                                  #   6, 1 PulseSequenceName
```

do you have private data which would attest to that field being useful  on XA** data?
bpinsard: (Sorry for the super late reply, slammed with 10**10 things since I got back from vacations.)

My bad for posting the wrong repo: https://github.com/neurolabusc/dcm_qa_xa30  contains XA dicoms with similar problem: only `PulseSequenceName` containing the pertinent info for seqinfo (and reflected in dcm2niix sidecars). 

I try to rely as much as possible on sequence info rather than idiosyncratic `ProtocolName` when I write heuristics, but yes it should be covered by tests. 
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/nipy/heudiconv/pull/753?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipy) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 82.05%. Comparing base [(`41b6d83`)](https://app.codecov.io/gh/nipy/heudiconv/commit/41b6d83d16bf35e701f1d86cf80d21abc81b3aa5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipy) to head [(`49c5783`)](https://app.codecov.io/gh/nipy/heudiconv/commit/49c57834594796c9e4acb4bc0dc7b2179a151a40?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipy).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master     #753      +/-   ##
==========================================
- Coverage   82.08%   82.05%   -0.04%     
==========================================
  Files          42       42              
  Lines        4215     4223       +8     
==========================================
+ Hits         3460     3465       +5     
- Misses        755      758       +3     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/nipy/heudiconv/pull/753?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipy).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=nipy).

bpinsard: Added really basic test that grabs all the dicoms in the test data folder and try to create seqinfo for each one.
It effectively covers the current fix.
As `create_seqinfo` was not covered at all in unit tests, I caught and fixed another problem with ProtocolName. 
@yarikoptic let me know if that's enough testing for that PR.
Thanks! 

Patch:
diff --git a/heudiconv/dicoms.py b/heudiconv/dicoms.py
index 1d276e3..6210f83 100644
--- a/heudiconv/dicoms.py
+++ b/heudiconv/dicoms.py
@@ -92,6 +92,7 @@ def create_seqinfo(
     image_type = get_typed_attr(dcminfo, "ImageType", tuple, ())
     is_moco = "MOCO" in image_type
     series_desc = get_typed_attr(dcminfo, "SeriesDescription", str, "")
+    protocol_name = get_typed_attr(dcminfo, "ProtocolName", str, "")
 
     if dcminfo.get([0x18, 0x24]):
         # GE and Philips
@@ -99,6 +100,9 @@ def create_seqinfo(
     elif dcminfo.get([0x19, 0x109C]):
         # Siemens
         sequence_name = dcminfo[0x19, 0x109C].value
+    elif dcminfo.get([0x18, 0x9005]):
+        # Siemens XA
+        sequence_name = dcminfo[0x18, 0x9005].value
     else:
         sequence_name = ""
 
@@ -133,7 +137,7 @@ def create_seqinfo(
         dim4=size[3],
         TR=TR,
         TE=TE,
-        protocol_name=dcminfo.ProtocolName,
+        protocol_name=protocol_name,
         is_motion_corrected=is_moco,
         is_derived="derived" in [x.lower() for x in image_type],
         patient_id=dcminfo.get("PatientID"),


Test patch:
diff --git a/heudiconv/tests/test_dicoms.py b/heudiconv/tests/test_dicoms.py
index 4cb2829..092019b 100644
--- a/heudiconv/tests/test_dicoms.py
+++ b/heudiconv/tests/test_dicoms.py
@@ -12,6 +12,8 @@ import pytest
 from heudiconv.cli.run import main as runner
 from heudiconv.convert import nipype_convert
 from heudiconv.dicoms import (
+    create_seqinfo,
+    dw,
     embed_dicom_and_nifti_metadata,
     get_datetime_from_dcm,
     get_reproducible_int,
@@ -178,6 +180,23 @@ def test_get_datetime_from_dcm_wo_dt() -> None:
     assert get_datetime_from_dcm(XA30_enhanced_dcm) is None
 
 
+dicom_test_data = [
+    (dw.wrapper_from_file(d_file), [d_file], op.basename(d_file))
+    for d_file in glob(op.join(TESTS_DATA_PATH, "*.dcm"))
+]
+
+
+@pytest.mark.parametrize("mw,series_files,series_id", dicom_test_data)
+def test_create_seqinfo(
+    mw: dw.Wrapper,
+    series_files: list[str],
+    series_id: str,
+) -> None:
+    seqinfo = create_seqinfo(mw, series_files, series_id)
+    assert seqinfo.sequence_name != ""
+    pass
+
+
 def test_get_reproducible_int() -> None:
     dcmfile = op.join(TESTS_DATA_PATH, "phantom.dcm")