Repository: pydicom/pydicom

Issue:
Codify not generating content sequences correctly
**Describe the bug**
I am trying to generate a radiation dose structure report. I ran Codify on an existing RDSR to generate a template. The sequence content is reproduced but does not seem to be attached  to the base dataset. When I run the generated python file the dicom file it saves has no sequence content information.

**Expected behavior**
I expect the dicom file generated by the python code from Codify to be similar to the original file.

**Steps To Reproduce**
$ python codify X-RayRadiationDoseReport001_ESR.dcm rdsr.py
$ python rsdr.py

I am not able to attached the above files but can supply them.

**Your environment**
module       | version
------       | -------
platform     | Linux-5.18.7-200.fc36.x86_64-x86_64-with-glibc2.35
Python       | 3.10.5 (main, Jun  9 2022, 00:00:00) [GCC 12.1.1 20220507 (Red Hat 12.1.1-1)]
pydicom      | 2.3.0
gdcm         | _module not found_
jpeg_ls      | _module not found_
numpy        | 1.22.4
PIL          | 9.2.0
pylibjpeg    | _module not found_
openjpeg     | _module not found_
libjpeg      | _module not found_

Regards
Alan

Discussion:
pep8speaks: Hello @darcymason! Thanks for opening this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



* In the file [`pydicom/util/codify.py`](https://github.com/pydicom/pydicom/blob/0988dc82e419e959d2f9ea4e83cafcbf9fc73a62/pydicom/util/codify.py):

> [Line 253:5](https://github.com/pydicom/pydicom/blob/0988dc82e419e959d2f9ea4e83cafcbf9fc73a62/pydicom/util/codify.py#L253): [E303](https://duckduckgo.com/?q=pep8%20E303) too many blank lines (2)


codecov[bot]: # [Codecov](https://codecov.io/gh/pydicom/pydicom/pull/1674?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) Report
> Merging [#1674](https://codecov.io/gh/pydicom/pydicom/pull/1674?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) (0988dc8) into [master](https://codecov.io/gh/pydicom/pydicom/commit/19b4739b525747b7ef6f4d88bd5447dea131af86?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) (19b4739) will **decrease** coverage by `9.34%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master    #1674      +/-   ##
==========================================
- Coverage   97.58%   88.23%   -9.35%     
==========================================
  Files          66       66              
  Lines       10712    10498     -214     
==========================================
- Hits        10453     9263    -1190     
- Misses        259     1235     +976     
```


| [Impacted Files](https://codecov.io/gh/pydicom/pydicom/pull/1674?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) | Coverage Δ | |
|---|---|---|
| [pydicom/util/codify.py](https://codecov.io/gh/pydicom/pydicom/pull/1674/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS91dGlsL2NvZGlmeS5weQ==) | `97.61% <100.00%> (+0.30%)` | :arrow_up: |
| [pydicom/pixel\_data\_handlers/util.py](https://codecov.io/gh/pydicom/pydicom/pull/1674/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS9waXhlbF9kYXRhX2hhbmRsZXJzL3V0aWwucHk=) | `17.93% <0.00%> (-82.07%)` | :arrow_down: |
| [pydicom/waveforms/numpy\_handler.py](https://codecov.io/gh/pydicom/pydicom/pull/1674/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS93YXZlZm9ybXMvbnVtcHlfaGFuZGxlci5weQ==) | `20.31% <0.00%> (-79.69%)` | :arrow_down: |
| [pydicom/encoders/gdcm.py](https://codecov.io/gh/pydicom/pydicom/pull/1674/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS9lbmNvZGVycy9nZGNtLnB5) | `20.96% <0.00%> (-75.81%)` | :arrow_down: |
| [pydicom/pixel\_data\_handlers/numpy\_handler.py](https://codecov.io/gh/pydicom/pydicom/pull/1674/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS9waXhlbF9kYXRhX2hhbmRsZXJzL251bXB5X2hhbmRsZXIucHk=) | `27.14% <0.00%> (-72.86%)` | :arrow_down: |
| [pydicom/pixel\_data\_handlers/gdcm\_handler.py](https://codecov.io/gh/pydicom/pydicom/pull/1674/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS9waXhlbF9kYXRhX2hhbmRsZXJzL2dkY21faGFuZGxlci5weQ==) | `26.22% <0.00%> (-66.46%)` | :arrow_down: |
| [pydicom/overlays/numpy\_handler.py](https://codecov.io/gh/pydicom/pydicom/pull/1674/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS9vdmVybGF5cy9udW1weV9oYW5kbGVyLnB5) | `35.18% <0.00%> (-64.82%)` | :arrow_down: |
| [pydicom/pixel\_data\_handlers/pillow\_handler.py](https://codecov.io/gh/pydicom/pydicom/pull/1674/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS9waXhlbF9kYXRhX2hhbmRsZXJzL3BpbGxvd19oYW5kbGVyLnB5) | `31.73% <0.00%> (-64.43%)` | :arrow_down: |
| [pydicom/pixel\_data\_handlers/rle\_handler.py](https://codecov.io/gh/pydicom/pydicom/pull/1674/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS9waXhlbF9kYXRhX2hhbmRsZXJzL3JsZV9oYW5kbGVyLnB5) | `36.97% <0.00%> (-63.03%)` | :arrow_down: |
| [pydicom/pixel\_data\_handlers/pylibjpeg\_handler.py](https://codecov.io/gh/pydicom/pydicom/pull/1674/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom#diff-cHlkaWNvbS9waXhlbF9kYXRhX2hhbmRsZXJzL3B5bGlianBlZ19oYW5kbGVyLnB5) | `40.74% <0.00%> (-57.41%)` | :arrow_down: |
| ... and [16 more](https://codecov.io/gh/pydicom/pydicom/pull/1674/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom) | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pydicom)

alanphys: From my side this appears to be working. I notice though that the group length tags are not in the generated DICOM file. It doesn't seem to affect the functioning of the file so I don't know how important it it.

Regards
Alan
mrbean-bremen: >  I notice though that the group length tags are not in the generated DICOM file

Group length tags are deprecated (except in the meta header), so we don't write them.
darcymason: > From my side this appears to be working

Glad to hear it.  I'm still ironing out a few things but should be able to finish this PR in the next couple of days.
darcymason: Okay, I think this is ready to merge - comments have been addressed.  Have added a couple of small things - allowing cli commands to find pydicom's charset files (which were used to add a test with unicode output), and printing the Python version when getting the `pydicom` command help (its otherwise hard to figure out which `pydicom` command is running when multiple Python versions are installed).

Patch:
diff --git a/doc/release_notes/v2.4.0.rst b/doc/release_notes/v2.4.0.rst
index 95edf705e..b11c4d0ec 100644
--- a/doc/release_notes/v2.4.0.rst
+++ b/doc/release_notes/v2.4.0.rst
@@ -7,6 +7,8 @@ Enhancements
 * Added attribute :attr:`~pydicom.tag.BaseTag.json_key` (:pr:`1648`)
 * Added value validation for numerical VRs, add type validation for all
   validated VRs (:issue:`1414`)
+* CLI commands now accept *pydicom* charset test files and CLI help shows
+  Python Version (:pr:`1674`)
 
 Fixes
 -----
@@ -15,3 +17,5 @@ Fixes
 * Increased download speed with progress bar for test data (:issue:`1611`)
 * Fixed crash due to invalid private creator (:issue:`1638`)
 * Fixed extremely long BytesLengthException error messages (:pr:`1683`)
+* In codify, ensure unique variable names for DICOM keywords repeated
+  in sequences, and handle unicode characters correctly (:issue:`1670`)
diff --git a/pydicom/cli/main.py b/pydicom/cli/main.py
index 967794f4f..a97ecd4d9 100644
--- a/pydicom/cli/main.py
+++ b/pydicom/cli/main.py
@@ -10,10 +10,11 @@ attributes, and calls set_defaults(func=callback_function)
 import argparse
 import pkg_resources
 import re
+import sys
 from typing import Tuple, cast, List, Any, Dict, Optional, Callable
 
 from pydicom import dcmread
-from pydicom.data.data_manager import get_testdata_file
+from pydicom.data.data_manager import get_charset_files, get_testdata_file
 from pydicom.dataset import Dataset
 
 
@@ -132,6 +133,14 @@ def filespec_parser(filespec: str) -> List[Tuple[Dataset, Any]]:
     except NotImplementedError:  # will get this if absolute path passed
         pydicom_filename = ""
 
+    # Check if filename is in charset files
+    if not pydicom_filename:
+        try:
+            char_filenames = get_charset_files(filename)
+            pydicom_filename = char_filenames[0]
+        except NotImplementedError:  # will get this if absolute path passed
+            pass
+
     if prefix == "pydicom":
         filename = pydicom_filename
 
@@ -201,8 +210,11 @@ def main(args: Optional[List[str]] = None) -> None:
     """
     global subparsers
 
+    py_version = sys.version.split()[0]
+
     parser = argparse.ArgumentParser(
-        prog="pydicom", description="pydicom command line utilities"
+        prog="pydicom",
+        description=f"pydicom command line utilities (Python {py_version})"
     )
     subparsers = parser.add_subparsers(help="subcommand help")
 
diff --git a/pydicom/util/codify.py b/pydicom/util/codify.py
index a93153ba6..748c66b11 100644
--- a/pydicom/util/codify.py
+++ b/pydicom/util/codify.py
@@ -18,7 +18,8 @@ import argparse
 import os.path
 import re
 import sys
-from typing import Optional, List, Callable
+from typing import Optional, List, Callable, cast
+from collections import deque
 
 import pydicom
 from pydicom.datadict import dictionary_keyword
@@ -80,7 +81,8 @@ def code_dataelem(
     dataelem: DataElement,
     dataset_name: str = "ds",
     exclude_size: Optional[int] = None,
-    include_private: bool = False
+    include_private: bool = False,
+    var_names: Optional[deque] = None
 ) -> str:
     """Code lines for a single DICOM data element
 
@@ -96,7 +98,8 @@ def code_dataelem(
         will only have a commented string for a value,
         causing a syntax error when the code is run,
         and thus prompting the user to remove or fix that line.
-
+    var_names: Union[deque, None]
+        Used internally to ensure unique variable names in nested sequences.
     Returns
     -------
     str
@@ -106,7 +109,8 @@ def code_dataelem(
 
     if dataelem.VR == VR.SQ:
         return code_sequence(
-            dataelem, dataset_name, exclude_size, include_private
+            dataelem, dataset_name, exclude_size, include_private,
+            var_names=var_names
         )
 
     # If in DICOM dictionary, set using the keyword
@@ -143,6 +147,7 @@ def code_sequence(
     exclude_size: Optional[int] = None,
     include_private: bool = False,
     name_filter: Callable[[str], str] = default_name_filter,
+    var_names: Optional[deque] = None,
 ) -> str:
     """Code lines for recreating a Sequence data element
 
@@ -162,6 +167,8 @@ def code_sequence(
     name_filter: Callable[[str], str]
         A callable taking a sequence name or sequence item name, and returning
         a shorter name for easier code reading
+    var_names: Union[deque, None]
+        Used internally to ensure unique variable names in nested sequences.
 
     Returns
     -------
@@ -169,6 +176,15 @@ def code_sequence(
         A string containing code lines to recreate a DICOM sequence
     """
 
+    # Normally var_names is given from code_dataset, but for some tests need
+    #   to initialize it
+    if var_names is None:
+        var_names = deque()
+
+    def unique_name(name: str) -> str:
+        name_count = cast(deque, var_names).count(name) - 1
+        return name if name_count == 0 else name + f"_{name_count}"
+
     lines = []
     seq = dataelem.value
     seq_name = dataelem.name
@@ -183,8 +199,11 @@ def code_sequence(
     lines.append("# " + seq_name)
 
     # Code line to create a new Sequence object
-    if name_filter:
-        seq_var = name_filter(seq_keyword)
+    seq_var = name_filter(seq_keyword)
+    var_names.append(seq_var)
+    orig_seq_var = seq_var
+    seq_var = unique_name(seq_var)
+
     lines.append(seq_var + " = Sequence()")
 
     # Code line to add the sequence to its parent
@@ -208,14 +227,29 @@ def code_sequence(
         lines.append("# " + seq_name + ": " + seq_item_name + " " + index_str)
 
         # Determine the variable name to use for the sequence item (dataset)
-        ds_name = seq_var.replace("_sequence", "") + index_str
+        ds_name = orig_seq_var.replace("_sequence", "") + index_str
 
-        # Code the sequence item
-        code_item = code_dataset(ds, ds_name, exclude_size, include_private)
-        lines.append(code_item)
+        # Append "_#" if name already in use (in parent sequences)
+        var_names.append(ds_name)
+        ds_name = unique_name(ds_name)
+
+        # Code the sequence item dataset
+        code_item = code_dataset(
+            ds, ds_name, exclude_size, include_private, var_names=var_names
+        )
 
-        # Code the line to append the item to its parent sequence
-        lines.append(seq_var + ".append(" + ds_name + ")")
+        # Remove variable name from stored list, this dataset complete
+        var_names.pop()
+
+        # Code dataset creation and appending that to sequence, then the rest
+        # This keeps the logic close together, rather than after many items set
+        code_split = code_item.splitlines()
+        lines.append(code_split[0])  # "<ds_name> = Dataset()"
+        lines.append(f"{seq_var}.append({ds_name})")
+        lines.extend(code_split[1:])
+
+    # Remove sequence variable name we've used
+    var_names.pop()
 
     # Join the lines and return a single string
     return line_term.join(lines)
@@ -227,6 +261,7 @@ def code_dataset(
     exclude_size: Optional[int] = None,
     include_private: bool = False,
     is_file_meta: bool = False,
+    var_names: Optional[deque] = None
 ) -> str:
     """Return Python code for creating `ds`.
 
@@ -245,6 +280,8 @@ def code_dataset(
         data elements will be coded.
     is_file_meta : bool, optional
         ``True`` if `ds` contains file meta information elements.
+    var_names: deque, optional
+        Used internally to ensure unique variable names in nested sequences.
 
     Returns
     -------
@@ -252,8 +289,12 @@ def code_dataset(
         The codified dataset.
     """
 
+    if var_names is None:
+        var_names = deque()
     lines = []
+
     ds_class = " = FileMetaDataset()" if is_file_meta else " = Dataset()"
+
     lines.append(dataset_name + ds_class)
     for dataelem in ds:
         # If a private data element and flag says so, skip it and go to next
@@ -261,7 +302,8 @@ def code_dataset(
             continue
         # Otherwise code the line and add it to the lines list
         code_line = code_dataelem(
-            dataelem, dataset_name, exclude_size, include_private
+            dataelem, dataset_name, exclude_size, include_private,
+            var_names=var_names
         )
         lines.append(code_line)
         # Add blank line if just coded a sequence
@@ -270,6 +312,7 @@ def code_dataset(
     # If sequence was end of this dataset, remove the extra blank line
     if len(lines) and lines[-1] == "":
         lines.pop()
+
     # Join all the code lines and return them
     return line_term.join(lines)
 
@@ -313,8 +356,8 @@ def code_file_from_dataset(
 
     Parameters
     ----------
-    filename : str
-        Complete path and filename of a DICOM file to convert
+    ds : Dataset
+        A pydicom Dataset to convert
     exclude_size : Union[int,None]
         If not None, values longer than this (in bytes)
         will only have a commented string for a value,
@@ -336,6 +379,7 @@ def code_file_from_dataset(
     filename = ds.get("filename")
     identifier = f"DICOM file '{filename}'" if filename else "non-file dataset"
 
+    lines.append("# -*- coding: utf-8 -*-")
     lines.append(f"# Coded version of {identifier}")
     lines.append("# Produced by pydicom codify utility script")
 
@@ -385,7 +429,7 @@ def set_parser_arguments(
     parser.add_argument(
         "outfile",
         nargs="?",
-        type=argparse.FileType("w"),
+        type=argparse.FileType("w", encoding="UTF-8"),
         help=(
             "Filename to write Python code to, if not specified then code is "
             "written to stdout"


Test patch:
diff --git a/pydicom/tests/test_cli.py b/pydicom/tests/test_cli.py
index b6be2f3b8..df53388b1 100644
--- a/pydicom/tests/test_cli.py
+++ b/pydicom/tests/test_cli.py
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 # Copyright 2020 pydicom authors. See LICENSE file for details.
 """Tests for command-line interface"""
 
@@ -138,6 +139,13 @@ class TestCLIcall:
         with pytest.raises(NotImplementedError):
             main("codify pydicom::rtplan.dcm::RTPlanLabel".split())
 
+    def test_codify_UTF8(self, capsys):
+        """CLI `codify` command creates code with utf-8 characters"""
+        main(f"codify pydicom::chrFren.dcm".split())
+        out, _ = capsys.readouterr()
+        assert out.startswith("# -*- coding: utf-8 -*-")
+        assert "Buc^Jérôme" in out
+
     def test_help(self, capsys):
         """CLI `help` command gives expected output"""
         # With subcommand
diff --git a/pydicom/tests/test_util.py b/pydicom/tests/test_util.py
index 20b3aa36e..caff06533 100644
--- a/pydicom/tests/test_util.py
+++ b/pydicom/tests/test_util.py
@@ -5,12 +5,11 @@ from contextlib import contextmanager
 
 import pytest
 
-from pydicom import config, dcmread
+from pydicom import config, dcmread, Dataset, Sequence
 from pydicom import filereader
 from pydicom._private_dict import private_dictionaries
 from pydicom.data import get_testdata_file
 from pydicom.dataelem import DataElement
-from pydicom.dataset import Dataset
 from pydicom.tag import Tag
 from pydicom.uid import (
     ImplicitVRLittleEndian, ExplicitVRBigEndian, ExplicitVRLittleEndian
@@ -23,6 +22,7 @@ from pydicom.util.codify import (
     default_name_filter,
     code_imports,
     code_dataelem,
+    code_dataset,
     main as codify_main,
 )
 from pydicom.util.dump import *
@@ -134,15 +134,34 @@ class TestCodify:
             "\n"
             "# Control Point Sequence: Control Point 1\n"
             "cp1 = Dataset()\n"
-            "cp1.PatientID = '1234'\n"
-            "cp_sequence.append(cp1)"
+            "cp_sequence.append(cp1)\n"
+            "cp1.PatientID = '1234'"
         )
 
         assert out == code_dataelem(elem)
 
-    def test_code_dataset(self):
-        """Test utils.codify.code_dataset"""
-        pass
+    def test_codify_recurring_keyword(self):
+        """Test utils.codify.code_dataset with same keyword nested"""
+        # Create fake Dataset with repeated DICOM keyword nested
+        # (0040, a730)  Content Sequence  1 item(s) ----
+        #    (0040, a040) Value Type                          CS: 'CODE'
+        #    (0040, a730)  Content Sequence  1 item(s) ----
+        #       (0040, a040) Value Type                          CS: 'CODE'
+
+        ds = Dataset()
+        ds.ContentSequence = seq1 = Sequence()
+        seq1.append(Dataset())
+        seq1[0].ValueType = "CODE"
+        seq1[0].ContentSequence = seq2 = Sequence()
+        seq2.append(Dataset())
+        seq2[0].ValueType = "CODE_1"
+        ds_code = code_dataset(ds)
+
+        # normal 1st use of var name
+        assert "content1.ValueType = 'CODE'" in ds_code
+
+        # Nested item of same name should have subscript
+        assert "content1_1.ValueType = 'CODE_1'" in ds_code
 
     def test_code_file(self, capsys):
         """Test utils.codify.code_file"""