Repository: pydicom/pydicom

Issue:
LUT Descriptor values don't follow standard
**Describe the bug**
(0028,3002) [LUT Descriptor](http://dicom.nema.org/medical/dicom/current/output/chtml/part03/sect_C.11.html#sect_C.11.1.1) has VM = 3, with value as `[number of entries in LUT, first stored pixel value mapped, LUT entry bit depth]`. The VR for the element is ambiguous and may be US or SS depending on the value of (0028,0103) Pixel Representation, however this only affects the second value, not the first or last which are always US.

The problem is that a Pixel Representation value of 1 (i.e. 2s complement) gives a LUT Descriptor value 1 as signed when it should always be unsigned.

> Since LUT Descriptor (0028,3002) is multi-valued, in an Explicit VR Transfer Syntax, only one value representation (US or SS) may be specified, even though the first and third values are always by definition interpreted as unsigned. The explicit VR actually used is dictated by the VR needed to represent the second value, which will be consistent with Pixel Representation (0028,0103).

Also affects Red/Green/Blue Palette Color Lookup Table Descriptor.

**Steps To Reproduce**
```python
from pydicom import dcmread
from pydicom.filebase import DicomBytesIO

# Explicit VR: SS
lut = b'\x28\x00\x02\x30\x53\x53\x06\x00\x00\xf5\x00\xf8\x10\x00'

bs = DicomBytesIO(lut)
bs.is_little_endian = True
bs.is_implicit_VR = False

ds = dcmread(bs, force=True)
assert ds.LUTDescriptor == [62720, -2048, 16]
```

**Your environment**
Affects current `master`


Discussion:

Patch:
diff --git a/doc/release_notes/v1.4.0.rst b/doc/release_notes/v1.4.0.rst
index 9db048c76..37d75109f 100644
--- a/doc/release_notes/v1.4.0.rst
+++ b/doc/release_notes/v1.4.0.rst
@@ -16,13 +16,15 @@ Fixes
 * Fixed assigning of empty values to data elements (:issue:`896`)
 * Fixed error in unpickling dataset (:issue:`947`)
 * 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`)
 
 Enhancements
 ............
 
 * Added support for converting (60xx,3000) *Overlay Data* to a numpy ndarray
   using :meth:`Dataset.overlay_array()
-  <pydicom.dataset.Dataset.overlay_array>` (issue:`912`)
+  <pydicom.dataset.Dataset.overlay_array>` (:issue:`912`)
 * Added support for deferred reading in file-like objects (:issue:`932`)
 * Tolerate values with multiple and/or incorrect padding bytes (:issue:`940`)
 * Added support for uncompressed pixel data with (0028,0004) *Photometric
diff --git a/pydicom/dataelem.py b/pydicom/dataelem.py
index ea33a31a4..8dfe3e2f2 100644
--- a/pydicom/dataelem.py
+++ b/pydicom/dataelem.py
@@ -686,6 +686,13 @@ RawDataElement = namedtuple('RawDataElement', msg)
 RawDataElement.is_raw = True
 
 
+# The first and third values of the following elements are always US
+#   even if the VR is SS (PS3.3 C.7.6.3.1.5, C.11.1, C.11.2).
+# (0028,1101-1103) RGB Palette Color LUT Descriptor
+# (0028,3002) LUT Descriptor
+_LUT_DESCRIPTOR_TAGS = (0x00281101, 0x00281102, 0x00281103, 0x00283002)
+
+
 def DataElement_from_raw(raw_data_element, encoding=None):
     """Return a :class:`DataElement` created from `raw_data_element`.
 
@@ -738,5 +745,10 @@ def DataElement_from_raw(raw_data_element, encoding=None):
         value = convert_value(VR, raw, encoding)
     except NotImplementedError as e:
         raise NotImplementedError("{0:s} in tag {1!r}".format(str(e), raw.tag))
+
+    if raw.tag in _LUT_DESCRIPTOR_TAGS and value[0] < 0:
+        # We only fix the first value as the third value is 8 or 16
+        value[0] += 65536
+
     return DataElement(raw.tag, VR, value, raw.value_tell,
                        raw.length == 0xFFFFFFFF, already_converted=True)
diff --git a/pydicom/pixel_data_handlers/util.py b/pydicom/pixel_data_handlers/util.py
index a20349de9..101f0eed8 100644
--- a/pydicom/pixel_data_handlers/util.py
+++ b/pydicom/pixel_data_handlers/util.py
@@ -115,9 +115,6 @@ def apply_color_lut(arr, ds=None, palette=None):
     lut_desc = ds.RedPaletteColorLookupTableDescriptor
     # A value of 0 = 2^16 entries
     nr_entries = lut_desc[0] or 2**16
-    # Workaround for #942: first value is always unsigned
-    if nr_entries < 0:
-        nr_entries += 2**16
 
     # May be negative if Pixel Representation is 1
     first_map = lut_desc[1]
@@ -217,9 +214,6 @@ def apply_modality_lut(arr, ds):
     if hasattr(ds, 'ModalityLUTSequence'):
         item = ds.ModalityLUTSequence[0]
         nr_entries = item.LUTDescriptor[0] or 2**16
-        # Workaround for #942: first value is always unsigned
-        if nr_entries < 0:
-            nr_entries += 2**16
         first_map = item.LUTDescriptor[1]
         nominal_depth = item.LUTDescriptor[2]
 


Test patch:
diff --git a/pydicom/tests/test_filereader.py b/pydicom/tests/test_filereader.py
index ad3f350d0..ab1f0dfd2 100644
--- a/pydicom/tests/test_filereader.py
+++ b/pydicom/tests/test_filereader.py
@@ -7,6 +7,7 @@ import io
 from io import BytesIO
 import os
 import shutil
+from struct import unpack
 import sys
 import tempfile
 
@@ -631,6 +632,27 @@ class TestReader(object):
         except EOFError:
             self.fail('Unexpected EOFError raised')
 
+    def test_lut_descriptor(self):
+        """Regression test for #942: incorrect first value"""
+        prefixes = [
+            b'\x28\x00\x01\x11',
+            b'\x28\x00\x02\x11',
+            b'\x28\x00\x03\x11',
+            b'\x28\x00\x02\x30'
+        ]
+        suffix = b'\x53\x53\x06\x00\x00\xf5\x00\xf8\x10\x00'
+
+        for raw_tag in prefixes:
+            tag = unpack('<2H', raw_tag)
+            bs = DicomBytesIO(raw_tag + suffix)
+            bs.is_little_endian = True
+            bs.is_implicit_VR = False
+
+            ds = dcmread(bs, force=True)
+            elem = ds[tag]
+            assert elem.VR == 'SS'
+            assert elem.value == [62720, -2048, 16]
+
 
 class TestIncorrectVR(object):
     def setup(self):
diff --git a/pydicom/tests/test_handler_util.py b/pydicom/tests/test_handler_util.py
index aeed99bdd..16314f064 100644
--- a/pydicom/tests/test_handler_util.py
+++ b/pydicom/tests/test_handler_util.py
@@ -841,18 +841,6 @@ class TestNumpy_ModalityLUT(object):
         out = apply_modality_lut(arr, ds)
         assert [0, 0, 0, 1] == list(out)
 
-    def test_lut_sequence_entries_negative(self):
-        """Test workaround for #942: SS VR should give uint nr entries."""
-        ds = dcmread(MOD_16_SEQ)
-        seq = ds.ModalityLUTSequence[0]
-        seq.LUTDescriptor = [-32767, 0, 16]  # 32769
-        seq.LUTData = [0] * 32768 + [1]
-        arr = np.asarray([-10, 0, 32767, 32768, 32769])
-        out = apply_modality_lut(arr, ds)
-        # IV < index 0 -> 0
-        # IV > index 32768 -> 32768
-        assert [0, 0, 0, 1, 1] == list(out)
-
     def test_unchanged(self):
         """Test no modality LUT transform."""
         ds = dcmread(MOD_16)
@@ -1154,26 +1142,6 @@ class TestNumpy_PaletteColor(object):
         assert [60160, 25600, 37376] == list(rgb[arr == 130][0])
         assert ([60160, 25600, 37376] == rgb[arr == 130]).all()
 
-    def test_nr_entries_negative(self):
-        """Test workaround for #942: SS VR should give uint nr entries."""
-        ds = dcmread(PAL_08_200_0_16_1F, force=True)
-        ds.file_meta = Dataset()
-        ds.file_meta.TransferSyntaxUID = ImplicitVRLittleEndian
-        ds.RedPaletteColorLookupTableDescriptor[0] = -32767  # 32769
-        # 16-bit entries, 32769 entries per LUT
-        ds.RedPaletteColorLookupTableData = b'\x00\x00' * 32768 + b'\xff\xff'
-        ds.GreenPaletteColorLookupTableData = b'\x00\x00' * 32768 + b'\xff\xff'
-        ds.BluePaletteColorLookupTableData = b'\x00\x00' * 32768 + b'\xff\xff'
-        # IV < index 0 -> 0
-        # IV > index 32768 -> 32768
-        arr = np.asarray([-10, 0, 32767, 32768, 32769])
-        rgb = apply_color_lut(arr, ds)
-        assert [0, 0, 0] == list(rgb[0])
-        assert [0, 0, 0] == list(rgb[1])
-        assert [0, 0, 0] == list(rgb[2])
-        assert [65535, 65535, 65535] == list(rgb[3])
-        assert [65535, 65535, 65535] == list(rgb[4])
-
 
 @pytest.mark.skipif(not HAVE_NP, reason="Numpy is not available")
 class TestNumpy_ExpandSegmentedLUT(object):