Repository: pydicom/pydicom

Issue:
Encoding to ISO 2022 IR 159 doesn't work
<!-- Instructions For Filing a Bug: https://github.com/pydicom/pydicom/blob/master/CONTRIBUTING.md#filing-bugs -->

#### Description
Encoding to ISO 2022 IR 159 doesn't work even if 'ISO 2022 IR 159' is passed to pydicom.charset.convert_encodings.

#### Steps/Code to Reproduce
ISO 2022 IR 159 is designed as supplement characters to ISO 2022 IR 87. So these characters are not frequent use. But person name sometimes contains them. In the following example, the letter of "鷗" is only in ISO 2022 IR 159. But we cannot encode them correctly. 

```
import pydicom

japanese_pn = u"Mori^Ogai=森^鷗外=もり^おうがい"
specific_character_sets = ["ISO 2022 IR 6", "ISO 2022 IR 87", "ISO 2022 IR 159"]
expect_encoded = (
    b"\x4d\x6f\x72\x69\x5e\x4f\x67\x61\x69\x3d\x1b\x24\x42\x3f"
    b"\x39\x1b\x28\x42\x5e\x1b\x24\x28\x44\x6c\x3f\x1b\x24\x42"
    b"\x33\x30\x1b\x28\x42\x3d\x1b\x24\x42\x24\x62\x24\x6a\x1b"
    b"\x28\x42\x5e\x1b\x24\x42\x24\x2a\x24\x26\x24\x2c\x24\x24"
    b"\x1b\x28\x42"
)

python_encodings = pydicom.charset.convert_encodings(specific_character_sets)
actual_encoded = pydicom.charset.encode_string(japanese_pn, python_encodings)

print("actual:{}".format(actual_encoded))
print("expect:{}".format(expect_encoded))
```
#### Expected Results
<!-- Please paste or describe the expected results.
Example: No error is thrown and the name of the patient is printed.-->
```
b'Mori^Ogai=\x1b$B?9\x1b(B^\x1b$(Dl?\x1b$B30\x1b(B=\x1b$B$b$j\x1b(B^\x1b$B$*$&$,$$\x1b(B'
```
#### Actual Results
<!-- Please paste or specifically describe the actual output or traceback.
(Use %xmode to deactivate ipython's trace beautifier)
Example: ```AttributeError: 'FileDataset' object has no attribute 'PatientID'```
-->
```
b'Mori^Ogai=?^??=??^????'
```

And the followin exception occurs.

```
/PATH/TO/MY/PYTHON/PACKAGES/pydicom/charset.py:488: UserWarning: Failed to encode value with encodings: iso8859, iso2022_jp, iso-2022-jp - using replacement characters in encoded string
  .format(', '.join(encodings)))
```

#### Versions
<!--
Please run the following snippet and paste the output below.
import platform; print(platform.platform())
import sys; print("Python", sys.version)
import pydicom; print("pydicom", pydicom.__version__)
-->
```
Linux-4.15.0-55-generic-x86_64-with-debian-buster-sid
Python 3.6.9 |Anaconda, Inc.| (default, Jul 30 2019, 19:07:31)
[GCC 7.3.0]
pydicom 1.3.0
```


<!-- Thanks for contributing! -->


Discussion:
pep8speaks: Hello @ar90n! 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/charset.py`](https://github.com/pydicom/pydicom/blob/5aa9d95fdb66be253a3469a4cf7d8672cd17d741/pydicom/charset.py):

> [Line 177:1](https://github.com/pydicom/pydicom/blob/5aa9d95fdb66be253a3469a4cf7d8672cd17d741/pydicom/charset.py#L177): [E302](https://duckduckgo.com/?q=pep8%20E302) expected 2 blank lines, found 1
> [Line 197:80](https://github.com/pydicom/pydicom/blob/5aa9d95fdb66be253a3469a4cf7d8672cd17d741/pydicom/charset.py#L197): [E501](https://duckduckgo.com/?q=pep8%20E501) line too long (88 > 79 characters)
> [Line 202:80](https://github.com/pydicom/pydicom/blob/5aa9d95fdb66be253a3469a4cf7d8672cd17d741/pydicom/charset.py#L202): [E501](https://duckduckgo.com/?q=pep8%20E501) line too long (80 > 79 characters)

* In the file [`pydicom/tests/test_charset.py`](https://github.com/pydicom/pydicom/blob/5aa9d95fdb66be253a3469a4cf7d8672cd17d741/pydicom/tests/test_charset.py):

> [Line 441:80](https://github.com/pydicom/pydicom/blob/5aa9d95fdb66be253a3469a4cf7d8672cd17d741/pydicom/tests/test_charset.py#L441): [E501](https://duckduckgo.com/?q=pep8%20E501) line too long (92 > 79 characters)
> [Line 442:80](https://github.com/pydicom/pydicom/blob/5aa9d95fdb66be253a3469a4cf7d8672cd17d741/pydicom/tests/test_charset.py#L442): [E501](https://duckduckgo.com/?q=pep8%20E501) line too long (84 > 79 characters)



Patch:
diff --git a/doc/release_notes/v1.4.0.rst b/doc/release_notes/v1.4.0.rst
index 37d75109f..1d39ad141 100644
--- a/doc/release_notes/v1.4.0.rst
+++ b/doc/release_notes/v1.4.0.rst
@@ -18,6 +18,7 @@ Fixes
 * Fixed error in pickling modified datasets (:issue:`951`)
 * Fixed improper conversion of the first value of the *LUT
   Descriptor* elements (0028,1101-1103) and (0028,3002) (:issue:`942`)
+* Fixed handling of ISO IR 159 encoding (:issue: `917`)
 
 Enhancements
 ............
diff --git a/pydicom/charset.py b/pydicom/charset.py
index aaec9e1d7..7cb73dd4e 100644
--- a/pydicom/charset.py
+++ b/pydicom/charset.py
@@ -45,7 +45,7 @@ python_encoding = {
     'ISO 2022 IR 144': 'iso_ir_144',
     'ISO 2022 IR 148': 'iso_ir_148',
     'ISO 2022 IR 149': 'euc_kr',
-    'ISO 2022 IR 159': 'iso-2022-jp',
+    'ISO 2022 IR 159': 'iso2022_jp_2',
     'ISO 2022 IR 166': 'iso_ir_166',
     'ISO 2022 IR 58': 'iso_ir_58',
     'ISO_IR 192': 'UTF8',  # from Chinese example, 2008 PS3.5 Annex J p1-4
@@ -81,7 +81,7 @@ CODES_TO_ENCODINGS = {
     ESC + b'-M': 'iso_ir_148',
     ESC + b'-T': 'iso_ir_166',
     ESC + b'$)C': 'euc_kr',
-    ESC + b'$(D': 'iso-2022-jp',
+    ESC + b'$(D': 'iso2022_jp_2',
     ESC + b'$)A': 'iso_ir_58',
 }
 
@@ -92,7 +92,7 @@ ENCODINGS_TO_CODES['shift_jis'] = ESC + b')I'
 # To decode them, the escape sequence shall be preserved in the input byte
 # string, and will be removed during decoding by Python.
 handled_encodings = ('iso2022_jp',
-                     'iso-2022-jp',
+                     'iso2022_jp_2',
                      'iso_ir_58')
 
 
@@ -165,10 +165,18 @@ def _encode_to_jis_x_0201(value, errors='strict'):
 
     return encoded
 
-
 def _encode_to_jis_x_0208(value, errors='strict'):
-    """Convert a unicode string into JIS X 0208 byte string using iso2022_jp
-    encodings.
+    """Convert a unicode string into JIS X 0208 byte string."""
+    return _encode_to_given_charset(value, 'ISO 2022 IR 87', errors=errors)
+
+
+def _encode_to_jis_x_0212(value, errors='strict'):
+    """Convert a unicode string into JIS X 0212 byte string."""
+    return _encode_to_given_charset(value, 'ISO 2022 IR 159', errors=errors)
+
+
+def _encode_to_given_charset(value, character_set, errors='strict'):
+    """Convert a unicode string into given character set.
     The escape sequence which is located at the end of the encoded value has
     to vary depending on the value 1 of SpecificCharacterSet. So we have to
     trim it and append the correct escape sequence manually.
@@ -177,6 +185,8 @@ def _encode_to_jis_x_0208(value, errors='strict'):
     ----------
     value : text type
         The unicode string as presented to the user.
+    character_set: str:
+        Character set for result.
     errors : str
         The behavior of a character which could not be encoded. This value
         is passed to errors argument of str.encode().
@@ -185,28 +195,30 @@ def _encode_to_jis_x_0208(value, errors='strict'):
     -------
     byte string
         The encoded string. If some characters in value could not be encoded to
-        JIS X 0208, it depends on the behavior of iso2022_jp encoder.
+        given character_set, it depends on the behavior of corresponding python
+        encoder.
 
     Raises
     ------
     UnicodeEncodeError
         If errors is set to 'strict' and `value` could not be encoded with
-        JIS X 0208.
+        given character_set.
     """
 
+    encoding = python_encoding[character_set]
     # If errors is not strict, this function is used as fallback.
     # So keep the tail escape sequence of encoded for backward compatibility.
     if errors != 'strict':
-        return value.encode('iso2022_jp', errors=errors)
+        return value.encode(encoding, errors=errors)
 
-    Encoder = codecs.getincrementalencoder('iso2022-jp')
+    Encoder = codecs.getincrementalencoder(encoding)
     encoder = Encoder()
 
     encoded = encoder.encode(value[0])
-    if encoded[:3] != ENCODINGS_TO_CODES['iso2022_jp']:
+    if not encoded.startswith(ENCODINGS_TO_CODES[encoding]):
         raise UnicodeEncodeError(
-            'iso2022_jp', value, 0, len(value),
-            'Given character is out of ISO IR 87')
+            encoding, value, 0, len(value),
+            'Given character is out of {}'.format(character_set))
 
     for i, c in enumerate(value[1:], 1):
         try:
@@ -215,10 +227,10 @@ def _encode_to_jis_x_0208(value, errors='strict'):
             e.start = i
             e.end = len(value)
             raise e
-        if b[:3] == ENCODINGS_TO_CODES['iso8859']:
+        if b[:1] == ESC:
             raise UnicodeEncodeError(
-                'iso2022_jp', value, i, len(value),
-                'Given character is out of ISO IR 87')
+                encoding, value, i, len(value),
+                'Given character is out of {}'.format(character_set))
         encoded += b
     return encoded
 
@@ -262,13 +274,13 @@ def _get_escape_sequence_for_encoding(encoding, encoded=None):
 
 
 # These encodings need escape sequence to handle alphanumeric characters.
-need_tail_escape_sequence_encodings = ('iso2022_jp', 'iso-2022-jp')
+need_tail_escape_sequence_encodings = ('iso2022_jp', 'iso2022_jp_2')
 
 
 custom_encoders = {
     'shift_jis': _encode_to_jis_x_0201,
     'iso2022_jp': _encode_to_jis_x_0208,
-    'iso-2022-jp': _encode_to_jis_x_0208
+    'iso2022_jp_2': _encode_to_jis_x_0212
 }
 
 
@@ -562,7 +574,7 @@ def _encode_string_parts(value, encodings):
     # unencoded_part is empty - we are done, return the encoded string
     if best_encoding in need_tail_escape_sequence_encodings:
         encoded += _get_escape_sequence_for_encoding(encodings[0])
-    return encoded
+    return bytes(encoded)
 
 
 def _encode_string_impl(value, encoding, errors='strict'):
diff --git a/pydicom/pixel_data_handlers/pillow_handler.py b/pydicom/pixel_data_handlers/pillow_handler.py
index de823a871..c6f546583 100644
--- a/pydicom/pixel_data_handlers/pillow_handler.py
+++ b/pydicom/pixel_data_handlers/pillow_handler.py
@@ -23,8 +23,8 @@ except ImportError:
     HAVE_JPEG = False
     HAVE_JPEG2K = False
 
-import pydicom.encaps
-from pydicom.pixel_data_handlers.util import dtype_corrected_for_endianness
+from pydicom.encaps import defragment_data, decode_data_sequence
+from pydicom.pixel_data_handlers.util import pixel_dtype
 import pydicom.uid
 
 
@@ -69,7 +69,7 @@ def supports_transfer_syntax(transfer_syntax):
     return transfer_syntax in PillowSupportedTransferSyntaxes
 
 
-def needs_to_convert_to_RGB(dicom_dataset):
+def needs_to_convert_to_RGB(ds):
     """Return ``True`` if the *Pixel Data* should to be converted from YCbCr to
     RGB.
 
@@ -78,36 +78,40 @@ def needs_to_convert_to_RGB(dicom_dataset):
     return False
 
 
-def should_change_PhotometricInterpretation_to_RGB(dicom_dataset):
+def should_change_PhotometricInterpretation_to_RGB(ds):
     """Return ``True`` if the *Photometric Interpretation* should be changed
     to RGB.
 
     This affects JPEG transfer syntaxes.
     """
-    should_change = dicom_dataset.SamplesPerPixel == 3
+    should_change = ds.SamplesPerPixel == 3
     return False
 
 
-def get_pixeldata(dicom_dataset):
-    """Use Pillow to decompress compressed *Pixel Data*.
+def get_pixeldata(ds):
+    """Return a :class:`numpy.ndarray` of the *Pixel Data*.
+
+    Parameters
+    ----------
+    ds : Dataset
+        The :class:`Dataset` containing an Image Pixel module and the
+        *Pixel Data* to be decompressed and returned.
 
     Returns
     -------
     numpy.ndarray
-       The *Pixel Data* as a :class:`numpy.ndarray`.
+       The contents of (7FE0,0010) *Pixel Data* as a 1D array.
 
     Raises
     ------
     ImportError
         If Pillow is not available.
     NotImplementedError
-        if the transfer syntax is not supported
-    TypeError
-        if the pixel data type is unsupported
+        If the transfer syntax is not supported
     """
     logger.debug("Trying to use Pillow to read pixel array "
                  "(has pillow = %s)", HAVE_PIL)
-    transfer_syntax = dicom_dataset.file_meta.TransferSyntaxUID
+    transfer_syntax = ds.file_meta.TransferSyntaxUID
     if not HAVE_PIL:
         msg = ("The pillow package is required to use pixel_array for "
                "this transfer syntax {0}, and pillow could not be "
@@ -132,94 +136,38 @@ def get_pixeldata(dicom_dataset):
                .format(transfer_syntax.name))
         raise NotImplementedError(msg)
 
-    # Make NumPy format code, e.g. "uint16", "int32" etc
-    # from two pieces of info:
-    # dicom_dataset.PixelRepresentation -- 0 for unsigned, 1 for signed;
-    # dicom_dataset.BitsAllocated -- 8, 16, or 32
-    if dicom_dataset.PixelRepresentation == 0:
-        format_str = 'uint{}'.format(dicom_dataset.BitsAllocated)
-    elif dicom_dataset.PixelRepresentation == 1:
-        format_str = 'int{}'.format(dicom_dataset.BitsAllocated)
-    else:
-        format_str = 'bad_pixel_representation'
-    try:
-        numpy_format = numpy.dtype(format_str)
-    except TypeError:
-        msg = ("Data type not understood by NumPy: "
-               "format='{}', PixelRepresentation={}, "
-               "BitsAllocated={}".format(
-                   format_str,
-                   dicom_dataset.PixelRepresentation,
-                   dicom_dataset.BitsAllocated))
-        raise TypeError(msg)
-
-    numpy_format = dtype_corrected_for_endianness(
-        dicom_dataset.is_little_endian, numpy_format)
-
-    # decompress here
     if transfer_syntax in PillowJPEGTransferSyntaxes:
         logger.debug("This is a JPEG lossy format")
-        if dicom_dataset.BitsAllocated > 8:
+        if ds.BitsAllocated > 8:
             raise NotImplementedError("JPEG Lossy only supported if "
                                       "Bits Allocated = 8")
-        generic_jpeg_file_header = b''
-        frame_start_from = 0
     elif transfer_syntax in PillowJPEG2000TransferSyntaxes:
         logger.debug("This is a JPEG 2000 format")
-        generic_jpeg_file_header = b''
-        # generic_jpeg_file_header = b'\x00\x00\x00\x0C\x6A'
-        #     b'\x50\x20\x20\x0D\x0A\x87\x0A'
-        frame_start_from = 0
     else:
         logger.debug("This is a another pillow supported format")
-        generic_jpeg_file_header = b''
-        frame_start_from = 0
-
-    try:
-        UncompressedPixelData = bytearray()
-        if ('NumberOfFrames' in dicom_dataset and
-                dicom_dataset.NumberOfFrames > 1):
-            # multiple compressed frames
-            CompressedPixelDataSeq = \
-                pydicom.encaps.decode_data_sequence(
-                    dicom_dataset.PixelData)
-            for frame in CompressedPixelDataSeq:
-                data = generic_jpeg_file_header + \
-                    frame[frame_start_from:]
-                fio = io.BytesIO(data)
-                try:
-                    decompressed_image = Image.open(fio)
-                except IOError as e:
-                    raise NotImplementedError(e.strerror)
-                UncompressedPixelData.extend(decompressed_image.tobytes())
-        else:
-            # single compressed frame
-            pixel_data = pydicom.encaps.defragment_data(
-                dicom_dataset.PixelData)
-            pixel_data = generic_jpeg_file_header + \
-                pixel_data[frame_start_from:]
-            try:
-                fio = io.BytesIO(pixel_data)
-                decompressed_image = Image.open(fio)
-            except IOError as e:
-                raise NotImplementedError(e.strerror)
-            UncompressedPixelData.extend(decompressed_image.tobytes())
-    except Exception:
-        raise
-
-    logger.debug(
-        "Successfully read %s pixel bytes", len(UncompressedPixelData)
-    )
-
-    pixel_array = numpy.frombuffer(UncompressedPixelData, numpy_format)
-
-    if (transfer_syntax in
-            PillowJPEG2000TransferSyntaxes and
-            dicom_dataset.BitsStored == 16):
+
+    pixel_bytes = bytearray()
+    if getattr(ds, 'NumberOfFrames', 1) > 1:
+        # multiple compressed frames
+        for frame in decode_data_sequence(ds.PixelData):
+            decompressed_image = Image.open(io.BytesIO(frame))
+            pixel_bytes.extend(decompressed_image.tobytes())
+    else:
+        # single compressed frame
+        pixel_data = defragment_data(ds.PixelData)
+        decompressed_image = Image.open(io.BytesIO(pixel_data))
+        pixel_bytes.extend(decompressed_image.tobytes())
+
+    logger.debug("Successfully read %s pixel bytes", len(pixel_bytes))
+
+    arr = numpy.frombuffer(pixel_bytes, pixel_dtype(ds))
+
+    if (transfer_syntax in PillowJPEG2000TransferSyntaxes and
+            ds.BitsStored == 16):
         # WHY IS THIS EVEN NECESSARY??
-        pixel_array &= 0x7FFF
+        arr &= 0x7FFF
 
-    if should_change_PhotometricInterpretation_to_RGB(dicom_dataset):
-        dicom_dataset.PhotometricInterpretation = "RGB"
+    if should_change_PhotometricInterpretation_to_RGB(ds):
+        ds.PhotometricInterpretation = "RGB"
 
-    return pixel_array
+    return arr


Test patch:
diff --git a/pydicom/tests/test_charset.py b/pydicom/tests/test_charset.py
index 0cc341e45..7c5518302 100644
--- a/pydicom/tests/test_charset.py
+++ b/pydicom/tests/test_charset.py
@@ -9,6 +9,7 @@ from pydicom import dcmread, config
 from pydicom.data import get_charset_files, get_testdata_files
 from pydicom.dataelem import DataElement
 from pydicom.filebase import DicomBytesIO
+from pydicom.valuerep import PersonName3
 
 # The file names (without '.dcm' extension) of most of the character test
 # files, together with the respective decoded PatientName tag values.
@@ -197,9 +198,9 @@ class TestCharset(object):
                            b'\x1b$(D\xc4\xe9\xef\xed\xf5\xf3\xe9\xef\xf2')
 
         with pytest.warns(UserWarning, match='Failed to decode byte string '
-                                             'with encodings: iso-2022-jp'):
+                                             'with encodings: iso2022_jp_2'):
             pydicom.charset.decode_element(elem, ['ISO 2022 IR 159'])
-            assert u'����������' == elem.value
+            assert u'���������' == elem.value
 
     def test_bad_decoded_multi_byte_encoding_enforce_standard(self):
         """Test handling bad encoding for single encoding if
@@ -207,7 +208,7 @@ class TestCharset(object):
         config.enforce_valid_values = True
         elem = DataElement(0x00100010, 'PN',
                            b'\x1b$(D\xc4\xe9\xef\xed\xf5\xf3\xe9\xef\xf2')
-        msg = ("'iso2022_jp' codec can't decode bytes in position 0-3: "
+        msg = ("'iso2022_jp_2' codec can't decode byte 0xc4 in position 4: "
                "illegal multibyte sequence")
         with pytest.raises(UnicodeDecodeError, match=msg):
             pydicom.charset.decode_element(elem, ['ISO 2022 IR 159'])
@@ -435,11 +436,26 @@ class TestCharset(object):
             ds_out = dcmread(fp)
             assert original_string == ds_out.PatientName.original_string
 
+        japanese_pn = PersonName3(u"Mori^Ogai=森^鷗外=もり^おうがい")
+        pyencs = pydicom.charset.convert_encodings(["ISO 2022 IR 6",
+                                                    "ISO 2022 IR 87",
+                                                    "ISO 2022 IR 159"])
+        actual_encoded = bytes(japanese_pn.encode(pyencs))
+        expect_encoded = (
+            b"\x4d\x6f\x72\x69\x5e\x4f\x67\x61\x69\x3d\x1b\x24\x42\x3f"
+            b"\x39\x1b\x28\x42\x5e\x1b\x24\x28\x44\x6c\x3f\x1b\x24\x42"
+            b"\x33\x30\x1b\x28\x42\x3d\x1b\x24\x42\x24\x62\x24\x6a\x1b"
+            b"\x28\x42\x5e\x1b\x24\x42\x24\x2a\x24\x26\x24\x2c\x24\x24"
+            b"\x1b\x28\x42"
+        )
+        assert expect_encoded == actual_encoded
+
     def test_japanese_multi_byte_encoding(self):
         """Test japanese multi byte strings are correctly encoded."""
-        encoded = pydicom.charset.encode_string(u'あaｱア',
-                                                ['shift_jis', 'iso2022_jp'])
-        assert b'\x1b$B$"\x1b(Ja\x1b)I\xb1\x1b$B%"\x1b(J' == encoded
+        encoded = pydicom.charset.encode_string(u'あaｱア齩', ['shift_jis',
+                                                'iso2022_jp', 'iso2022_jp_2'])
+        expect = b'\x1b$B$"\x1b(Ja\x1b)I\xb1\x1b$B%"\x1b$(DmN\x1b(J'
+        assert expect == bytes(encoded)
 
     def test_bad_japanese_encoding(self):
         """Test japanese multi byte strings are not correctly encoded."""
diff --git a/pydicom/tests/test_pillow_pixel_data.py b/pydicom/tests/test_pillow_pixel_data.py
index 55514dfe0..683484f18 100644
--- a/pydicom/tests/test_pillow_pixel_data.py
+++ b/pydicom/tests/test_pillow_pixel_data.py
@@ -287,11 +287,6 @@ class Test_JPEG2000Tests_with_pillow(object):
         """Reset the pixel data handlers."""
         pydicom.config.pixel_data_handlers = self.original_handlers
 
-    def test_raises_if_endianess_not_set(self):
-        self.jpeg_2k_lossless.is_little_endian = None
-        with pytest.raises(ValueError):
-            _ = self.jpeg_2k_lossless.pixel_array
-
     def testJPEG2000(self):
         """Test reading element values works OK with pillow pixel handler."""
         # XX also tests multiple-valued AT data element
@@ -354,7 +349,8 @@ class Test_JPEGlossyTests_with_pillow(object):
 
     def testJPEGlossyPixelArray(self):
         """Test decoding JPEG lossy with pillow handler fails."""
-        with pytest.raises(NotImplementedError):
+        msg = r"JPEG Lossy only supported if Bits Allocated = 8"
+        with pytest.raises(NotImplementedError, match=msg):
             self.jpeg_lossy.pixel_array
 
     def testJPEGBaselineColor3DPixelArray(self):
@@ -608,5 +604,5 @@ class Test_JPEGlosslessTests_with_pillow(object):
 
     def testJPEGlosslessPixelArray(self):
         """Test decoding JPEG lossless with pillow handler fails."""
-        with pytest.raises(RuntimeError):
+        with pytest.raises(IOError, match=r"cannot identify image file"):
             self.jpeg_lossless.pixel_array