Repository: pydicom/pydicom

Issue:
Ambiguous VR element could be read in <=1.1.0 but is broken in >=1.2.0
<!-- Instructions For Filing a Bug: https://github.com/pydicom/pydicom/blob/master/CONTRIBUTING.md#filing-bugs -->

#### Description
Attribute Error thrown when printing (0x0028, 0x0120) PixelPaddingValue

#### Steps/Code to Reproduce
Using pydicom 1.2.2 and above (including master branch as of issue creation date):
```
from pydicom import dcmread

ds = dcmread('rtss.dcm')
ds

Exception in thread Thread-1:
Traceback (most recent call last):
  File "/Users/apanchal/Projects/venvs/dicom/lib/python3.7/site-packages/pydicom/filewriter.py", line 157, in correct_ambiguous_vr_element
    _correct_ambiguous_vr_element(elem, ds, is_little_endian)
  File "/Users/apanchal/Projects/venvs/dicom/lib/python3.7/site-packages/pydicom/filewriter.py", line 75, in _correct_ambiguous_vr_element
    if ds.PixelRepresentation == 0:
  File "/Users/apanchal/Projects/venvs/dicom/lib/python3.7/site-packages/pydicom/dataset.py", line 711, in __getattr__
    return super(Dataset, self).__getattribute__(name)
AttributeError: 'FileDataset' object has no attribute 'PixelRepresentation'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/threading.py", line 917, in _bootstrap_inner
    self.run()
  File "/usr/local/Cellar/python/3.7.0/Frameworks/Python.framework/Versions/3.7/lib/python3.7/threading.py", line 865, in run
    self._target(*self._args, **self._kwargs)
  File "/Users/apanchal/Projects/test.py", line 107, in processing_thread
    dp.ds, name, patientid, patientdob)
  File "/Users/apanchal/Projects/test.py", line 144, in UpdateElements
    for item in data:
  File "/Users/apanchal/Projects/venvs/dicom/lib/python3.7/site-packages/pydicom/dataset.py", line 1045, in __iter__
    yield self[tag]
  File "/Users/apanchal/Projects/venvs/dicom/lib/python3.7/site-packages/pydicom/dataset.py", line 805, in __getitem__
    self[tag], self, data_elem[6])
  File "/Users/apanchal/Projects/venvs/dicom/lib/python3.7/site-packages/pydicom/filewriter.py", line 161, in correct_ambiguous_vr_element
    raise AttributeError(reason)
AttributeError: Failed to resolve ambiguous VR for tag (0028, 0120): 'FileDataset' object has no attribute 'PixelRepresentation'
```

Anonymized RTSTRUCT file is attached: [RTSTRUCT.zip](https://github.com/pydicom/pydicom/files/3124625/RTSTRUCT.zip)

#### Expected Results
The dataset is printed. This worked in pydicom 1.1.0 and below.

Since `PixelRepresentation` is not defined in the dataset, this attribute cannot be printed anymore.

What's strange is that according to the standard PixelPaddingValue (0028, 0120) is 1C for RTSTRUCT, but in this file it has no other tags referencing PixelData. So it probably should not have been included by the vendor.

I am wondering if there should be another path like in #809 that can handle the missing PixelRepresentation attribute.

#### Actual Results
```AttributeError: Failed to resolve ambiguous VR for tag (0028, 0120): 'FileDataset' object has no attribute 'PixelRepresentation'```

#### Versions
```
Darwin-17.7.0-x86_64-i386-64bit
Python 3.7.0 (default, Jul 23 2018, 20:22:55) 
[Clang 9.1.0 (clang-902.0.39.2)]
pydicom 1.2.2
```

Discussion:

Patch:
diff --git a/README.md b/README.md
index e0fe70308..ea2f88310 100644
--- a/README.md
+++ b/README.md
@@ -7,7 +7,7 @@ pydicom
 [![codecov](https://codecov.io/gh/pydicom/pydicom/branch/master/graph/badge.svg)](https://codecov.io/gh/pydicom/pydicom)
 [![Python version](https://img.shields.io/pypi/pyversions/pydicom.svg)](https://img.shields.io/pypi/pyversions/pydicom.svg)
 [![PyPI version](https://badge.fury.io/py/pydicom.svg)](https://badge.fury.io/py/pydicom)
-[![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.1450666.svg)](https://doi.org/10.5281/zenodo.1450666)
+[![DOI](https://zenodo.org/badge/DOI/10.5281/zenodo.2541240.svg)](https://doi.org/10.5281/zenodo.2541240)
 
 pydicom is a pure python package for working with [DICOM](http://medical.nema.org/) files.
 It was made for inspecting and modifying DICOM data in an easy "pythonic" way.
diff --git a/doc/whatsnew/v1.3.0.rst b/doc/whatsnew/v1.3.0.rst
index 5675aa8dd..1f6074a8d 100644
--- a/doc/whatsnew/v1.3.0.rst
+++ b/doc/whatsnew/v1.3.0.rst
@@ -28,7 +28,9 @@ Enhancements
 
 Fixes
 .....
-
+* Do not raise while resolving an ambiguous VR dependent on
+  `PixelRepresentation` if both `PixelRepresentation` and `PixelData` are
+  not present (:issue:`838`)
 * Raise exception with specific message if value is too large to be written
   in explicit transfer syntax (:issue:`757`)
 * Do not derive `Dataset` from `dict` - fixes behavior in newer Python versions
diff --git a/pydicom/filewriter.py b/pydicom/filewriter.py
index 23ab57f09..c0bb8cb58 100644
--- a/pydicom/filewriter.py
+++ b/pydicom/filewriter.py
@@ -72,7 +72,10 @@ def _correct_ambiguous_vr_element(elem, ds, is_little_endian):
         # PixelRepresentation is usually set in the root dataset
         while 'PixelRepresentation' not in ds and ds.parent:
             ds = ds.parent
-        if ds.PixelRepresentation == 0:
+        # if no pixel data is present, none if these tags is used,
+        # so we can just ignore a missing PixelRepresentation in this case
+        if ('PixelRepresentation' not in ds and 'PixelData' not in ds or
+                ds.PixelRepresentation == 0):
             elem.VR = 'US'
             byte_type = 'H'
         else:


Test patch:
diff --git a/pydicom/tests/test_filewriter.py b/pydicom/tests/test_filewriter.py
index ea77c15ed..0a4ba3ee3 100644
--- a/pydicom/tests/test_filewriter.py
+++ b/pydicom/tests/test_filewriter.py
@@ -671,9 +671,15 @@ class TestCorrectAmbiguousVR(object):
         assert 1 == ds.SmallestValidPixelValue
         assert 'SS' == ds[0x00280104].VR
 
-        # If no PixelRepresentation AttributeError shall be raised
+        # If no PixelRepresentation and no PixelData is present 'US' is set
         ref_ds = Dataset()
         ref_ds.SmallestValidPixelValue = b'\x00\x01'  # Big endian 1
+        ds = correct_ambiguous_vr(deepcopy(ref_ds), True)
+        assert 'US' == ds[0x00280104].VR
+
+        # If no PixelRepresentation but PixelData is present
+        # AttributeError shall be raised
+        ref_ds.PixelData = b'123'
         with pytest.raises(AttributeError,
                            match=r"Failed to resolve ambiguous VR for tag "
                                  r"\(0028, 0104\):.* 'PixelRepresentation'"):
@@ -697,9 +703,14 @@ class TestCorrectAmbiguousVR(object):
         assert [256, 1, 16] == ds.LUTDescriptor
         assert 'SS' == ds[0x00283002].VR
 
-        # If no PixelRepresentation AttributeError shall be raised
+        # If no PixelRepresentation and no PixelData is present 'US' is set
         ref_ds = Dataset()
         ref_ds.LUTDescriptor = b'\x01\x00\x00\x01\x00\x10'
+        ds = correct_ambiguous_vr(deepcopy(ref_ds), True)
+        assert 'US' == ds[0x00283002].VR
+
+        # If no PixelRepresentation AttributeError shall be raised
+        ref_ds.PixelData = b'123'
         with pytest.raises(AttributeError,
                            match=r"Failed to resolve ambiguous VR for tag "
                                  r"\(0028, 3002\):.* 'PixelRepresentation'"):
@@ -928,6 +939,13 @@ class TestCorrectAmbiguousVRElement(object):
         """Test correct ambiguous US/SS element"""
         ds = Dataset()
         ds.PixelPaddingValue = b'\xfe\xff'
+        out = correct_ambiguous_vr_element(ds[0x00280120], ds, True)
+        # assume US if PixelData is not set
+        assert 'US' == out.VR
+
+        ds = Dataset()
+        ds.PixelPaddingValue = b'\xfe\xff'
+        ds.PixelData = b'3456'
         with pytest.raises(AttributeError,
                            match=r"Failed to resolve ambiguous VR for tag "
                                  r"\(0028, 0120\):.* 'PixelRepresentation'"):