Repository: pydicom/pydicom

Issue:
No encoding with write_PN raises TypeError
#### Description
`filewriter.write_PN()` with `encoding=None` (default) raises `TypeError`

#### Steps/Code to Reproduce
```python
>>> from pydicom.filebase import DicomBytesIO
>>> from pydicom.dataelem import DataElement
>>> from pydicom.filewriter import write_PN
>>> fp = DicomBytesIO()
>>> fp.is_little_endian = True
>>> elem = DataElement(0x00100010, 'PN', u'\u03b8')
>>> write_PN(fp, elem)
```
```
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "../pydicom/pydicom/filewriter.py", line 228, in write_PN
    val = [elem.encode(encoding) for elem in val]
TypeError: encode() argument 1 must be string, not list
```
Occurs because when `encoding=None`, `encoding = [default] * 3` is used.

I'm not sure what the impact is in practise since `write_PN` only seems to get called internally with encoding not set to None.

#### Versions
Python 2.7


Discussion:

Patch:
diff --git a/pydicom/dataset.py b/pydicom/dataset.py
index 1052b8587..8d12864bb 100644
--- a/pydicom/dataset.py
+++ b/pydicom/dataset.py
@@ -28,8 +28,8 @@ from pydicom.datadict import (tag_for_keyword, keyword_for_tag,
                               repeater_has_keyword)
 from pydicom.tag import Tag, BaseTag, tag_in_exception
 from pydicom.dataelem import DataElement, DataElement_from_raw, RawDataElement
-from pydicom.uid import (UncompressedPixelTransferSyntaxes, 
-						 ExplicitVRLittleEndian)
+from pydicom.uid import (UncompressedPixelTransferSyntaxes,
+                         ExplicitVRLittleEndian)
 import pydicom  # for dcmwrite
 import pydicom.charset
 from pydicom.config import logger
@@ -400,14 +400,13 @@ class Dataset(dict):
             return True
 
         if isinstance(other, self.__class__):
-            # Compare Elements using values() and class variables using
-            # __dict__
+            # Compare Elements using values()
             # Convert values() to a list for compatibility between
             #   python 2 and 3
             # Sort values() by element tag
             self_elem = sorted(list(self.values()), key=lambda x: x.tag)
             other_elem = sorted(list(other.values()), key=lambda x: x.tag)
-            return self_elem == other_elem and self.__dict__ == other.__dict__
+            return self_elem == other_elem
 
         return NotImplemented
 
@@ -697,9 +696,9 @@ class Dataset(dict):
 
         Returns
         -------
-        None  
+        None
             Converted pixel data is stored internally in the dataset.
-            
+
         If a compressed image format, the image is  decompressed,
         and any related data elements are changed accordingly.
         """
@@ -720,7 +719,9 @@ class Dataset(dict):
                     pixel_array = x.get_pixeldata(self)
                     self._pixel_array = self._reshape_pixel_array(pixel_array)
                     if x.needs_to_convert_to_RGB(self):
-                        self._pixel_array = self._convert_YBR_to_RGB(self._pixel_array)
+                        self._pixel_array = self._convert_YBR_to_RGB(
+                            self._pixel_array
+                        )
                     successfully_read_pixel_data = True
                     break
                 except Exception as e:
@@ -743,31 +744,31 @@ class Dataset(dict):
                     raise NotImplementedError(msg)
             # is this guaranteed to work if memory is re-used??
             self._pixel_id = id(self.PixelData)
-            
+
     def decompress(self):
         """Decompresses pixel data and modifies the Dataset in-place
 
-		If not a compressed tranfer syntax, then pixel data is converted
-		to a numpy array internally, but not returned.
-		
-		If compressed pixel data, then is decompressed using an image handler,
-		and internal state is updated appropriately:
-		    - TransferSyntax is updated to non-compressed form
-			- is_undefined_length for pixel data is set False
+        If not a compressed tranfer syntax, then pixel data is converted
+        to a numpy array internally, but not returned.
+
+        If compressed pixel data, then is decompressed using an image handler,
+        and internal state is updated appropriately:
+            - TransferSyntax is updated to non-compressed form
+            - is_undefined_length for pixel data is set False
 
         Returns
         -------
         None
 
-		Raises
+        Raises
         ------
         NotImplementedError
             If the pixel data was originally compressed but file is not
-			ExplicitVR LittleEndian as required by Dicom standard
-        """		
+            ExplicitVR LittleEndian as required by Dicom standard
+        """
         self.convert_pixel_data()
         self.is_decompressed = True
-		# May have been undefined length pixel data, but won't be now
+        # May have been undefined length pixel data, but won't be now
         if 'PixelData' in self:
             self[0x7fe00010].is_undefined_length = False
 
@@ -780,14 +781,13 @@ class Dataset(dict):
             # Check that current file as read does match expected
             if not self.is_little_endian or self.is_implicit_VR:
                 msg = ("Current dataset does not match expected ExplicitVR "
-                       "LittleEndian transfer syntax from a compressed " 
+                       "LittleEndian transfer syntax from a compressed "
                        "transfer syntax")
                 raise NotImplementedError(msg)
-                
+
             # All is as expected, updated the Transfer Syntax
             self.file_meta.TransferSyntaxUID = ExplicitVRLittleEndian
 
-
     @property
     def pixel_array(self):
         """Return the Pixel Data as a NumPy array.
@@ -1233,3 +1233,29 @@ class FileDataset(Dataset):
         if self.filename and os.path.exists(self.filename):
             statinfo = os.stat(self.filename)
             self.timestamp = statinfo.st_mtime
+
+    def __eq__(self, other):
+        """Compare `self` and `other` for equality.
+
+        Returns
+        -------
+        bool
+            The result if `self` and `other` are the same class
+        NotImplemented
+            If `other` is not the same class as `self` then returning
+            NotImplemented delegates the result to superclass.__eq__(subclass)
+        """
+        # When comparing against self this will be faster
+        if other is self:
+            return True
+
+        if isinstance(other, self.__class__):
+            # Compare Elements using values() and class members using __dict__
+            # Convert values() to a list for compatibility between
+            #   python 2 and 3
+            # Sort values() by element tag
+            self_elem = sorted(list(self.values()), key=lambda x: x.tag)
+            other_elem = sorted(list(other.values()), key=lambda x: x.tag)
+            return self_elem == other_elem and self.__dict__ == other.__dict__
+
+        return NotImplemented
diff --git a/pydicom/filewriter.py b/pydicom/filewriter.py
index ceb809915..01d9a4911 100644
--- a/pydicom/filewriter.py
+++ b/pydicom/filewriter.py
@@ -226,7 +226,10 @@ def write_PN(fp, data_element, padding=b' ', encoding=None):
         val = data_element.value
 
     if isinstance(val[0], compat.text_type) or not in_py2:
-        val = [elem.encode(encoding) for elem in val]
+        try:
+            val = [elem.encode(encoding) for elem in val]
+        except TypeError:
+            val = [elem.encode(encoding[0]) for elem in val]
 
     val = b'\\'.join(val)
 


Test patch:
diff --git a/pydicom/tests/test_dataset.py b/pydicom/tests/test_dataset.py
index fee7082bc..822b04e79 100644
--- a/pydicom/tests/test_dataset.py
+++ b/pydicom/tests/test_dataset.py
@@ -403,32 +403,35 @@ class DatasetTests(unittest.TestCase):
 
     def testEqualityNoSequence(self):
         """Dataset: equality returns correct value with simple dataset"""
+        # Test empty dataset
+        assert Dataset() == Dataset()
+
         d = Dataset()
         d.SOPInstanceUID = '1.2.3.4'
         d.PatientName = 'Test'
-        self.assertTrue(d == d)
+        assert d == d
 
         e = Dataset()
         e.PatientName = 'Test'
         e.SOPInstanceUID = '1.2.3.4'
-        self.assertTrue(d == e)
+        assert d == e
 
         e.SOPInstanceUID = '1.2.3.5'
-        self.assertFalse(d == e)
+        assert not d == e
 
         # Check VR
         del e.SOPInstanceUID
         e.add(DataElement(0x00080018, 'PN', '1.2.3.4'))
-        self.assertFalse(d == e)
+        assert not d == e
 
         # Check Tag
         del e.SOPInstanceUID
         e.StudyInstanceUID = '1.2.3.4'
-        self.assertFalse(d == e)
+        assert not d == e
 
         # Check missing Element in self
         e.SOPInstanceUID = '1.2.3.4'
-        self.assertFalse(d == e)
+        assert not d == e
 
         # Check missing Element in other
         d = Dataset()
@@ -437,7 +440,7 @@ class DatasetTests(unittest.TestCase):
 
         e = Dataset()
         e.SOPInstanceUID = '1.2.3.4'
-        self.assertFalse(d == e)
+        assert not d == e
 
     def testEqualityPrivate(self):
         """Dataset: equality returns correct value"""
@@ -500,16 +503,14 @@ class DatasetTests(unittest.TestCase):
 
     def testEqualityUnknown(self):
         """Dataset: equality returns correct value with extra members """
+        # Non-element class members are ignored in equality testing
         d = Dataset()
         d.SOPEustaceUID = '1.2.3.4'
-        self.assertTrue(d == d)
+        assert d == d
 
         e = Dataset()
-        e.SOPEustaceUID = '1.2.3.4'
-        self.assertTrue(d == e)
-
         e.SOPEustaceUID = '1.2.3.5'
-        self.assertFalse(d == e)
+        assert d == e
 
     def testEqualityInheritance(self):
         """Dataset: equality returns correct value for subclass """
@@ -529,6 +530,19 @@ class DatasetTests(unittest.TestCase):
         self.assertFalse(d == e)
         self.assertFalse(e == d)
 
+    def test_equality_elements(self):
+        """Test that Dataset equality only checks DataElements."""
+        d = Dataset()
+        d.SOPInstanceUID = '1.2.3.4'
+        d.PatientName = 'Test'
+        d.foo = 'foo'
+        assert d == d
+
+        e = Dataset()
+        e.PatientName = 'Test'
+        e.SOPInstanceUID = '1.2.3.4'
+        assert d == e
+
     def test_inequality(self):
         """Test inequality operator"""
         d = Dataset()
diff --git a/pydicom/tests/test_filewriter.py b/pydicom/tests/test_filewriter.py
index 9ed2347ea..6e829300b 100644
--- a/pydicom/tests/test_filewriter.py
+++ b/pydicom/tests/test_filewriter.py
@@ -1826,7 +1826,6 @@ class TestWriteNumbers(object):
 
 class TestWritePN(object):
     """Test filewriter.write_PN"""
-    @pytest.mark.skip("Raises exception due to issue #489")
     def test_no_encoding_unicode(self):
         """If PN element has no encoding info, default is used"""
         fp = DicomBytesIO()
@@ -1979,12 +1978,11 @@ class TestWriteNumbers(object):
 
 class TestWritePN(object):
     """Test filewriter.write_PN"""
-    @pytest.mark.skip("Raises exception due to issue #489")
     def test_no_encoding_unicode(self):
         """If PN element as no encoding info, default is used"""
         fp = DicomBytesIO()
         fp.is_little_endian = True
-        elem = DataElement(0x00100010, 'PN', u'\u03b8')
+        elem = DataElement(0x00100010, 'PN', u'\u00e8')
         write_PN(fp, elem)
 
     def test_no_encoding(self):