Repository: pydicom/pydicom

Issue:
Encoding type lost with PersonName3
#### Description
Encodings lost when converting DataElements that use PersonName3

#### Steps/Code to Reproduce
```python
from pydicom.charset import decode
from pydicom.dataelem import DataElement

elem = DataElement(0x00100010, 'PN', 'Test')
decode(elem, ['ISO 2022 IR 126'])
assert 'iso_ir_126' in elem.values.encodings
```
#### Actual Results
elem.values.encodings is set to ['iso8859', 'iso8859', 'iso8859'] because `charset.decode` calls
`data_element.value = data_element.value.decode(encodings)`. The `DataElement.value` setter then converts the `PersonName3` object with the correct encodings into a new `PersonName3` object with default encodings.

#### Versions
python 3



Discussion:

Patch:
diff --git a/pydicom/uid.py b/pydicom/uid.py
index 483159e47..741450672 100644
--- a/pydicom/uid.py
+++ b/pydicom/uid.py
@@ -162,15 +162,16 @@ class UID(str):
     @property
     def name(self):
         """Return the UID name from the UID dictionary."""
-        if self in UID_dictionary:
+        uid_string = str.__str__(self)
+        if uid_string in UID_dictionary:
             return UID_dictionary[self][0]
 
-        return str.__str__(self)
+        return uid_string
 
     @property
     def type(self):
         """Return the UID type from the UID dictionary."""
-        if self in UID_dictionary:
+        if str.__str__(self) in UID_dictionary:
             return UID_dictionary[self][1]
 
         return ''
@@ -178,7 +179,7 @@ class UID(str):
     @property
     def info(self):
         """Return the UID info from the UID dictionary."""
-        if self in UID_dictionary:
+        if str.__str__(self) in UID_dictionary:
             return UID_dictionary[self][2]
 
         return ''
@@ -186,7 +187,7 @@ class UID(str):
     @property
     def is_retired(self):
         """Return True if the UID is retired, False otherwise or if private."""
-        if self in UID_dictionary:
+        if str.__str__(self) in UID_dictionary:
             return bool(UID_dictionary[self][3])
 
         return False
diff --git a/pydicom/valuerep.py b/pydicom/valuerep.py
index 3f07ef740..1521c16e1 100644
--- a/pydicom/valuerep.py
+++ b/pydicom/valuerep.py
@@ -517,6 +517,7 @@ def MultiString(val, valtype=str):
 class PersonName3(object):
     def __init__(self, val, encodings=default_encoding):
         if isinstance(val, PersonName3):
+            encodings = val.encodings
             val = val.original_string
 
         self.original_string = val


Test patch:
diff --git a/pydicom/tests/test_charset.py b/pydicom/tests/test_charset.py
index 7cb62bd75..bb8f838f5 100644
--- a/pydicom/tests/test_charset.py
+++ b/pydicom/tests/test_charset.py
@@ -10,6 +10,7 @@ import pydicom.charset
 from pydicom import dicomio
 from pydicom.data import get_charset_files
 from pydicom.data import get_testdata_files
+from pydicom.dataelem import DataElement
 
 latin1_file = get_charset_files("chrFren.dcm")[0]
 jp_file = get_charset_files("chrH31.dcm")[0]
@@ -85,6 +86,18 @@ class charsetTests(unittest.TestCase):
                     '\033$B$d$^$@\033(B^\033$B$?$m$&\033(B')
         self.assertEqual(expected, ds.PatientName)
 
+    def test_bad_charset(self):
+        """Test bad charset defaults to ISO IR 6"""
+        # Python 3: elem.value is PersonName3, Python 2: elem.value is str
+        elem = DataElement(0x00100010, 'PN', 'CITIZEN')
+        pydicom.charset.decode(elem, ['ISO 2022 IR 126'])
+        # After decode Python 2: elem.value is PersonNameUnicode
+        assert 'iso_ir_126' in elem.value.encodings
+        assert 'iso8859' not in elem.value.encodings
+        # default encoding is iso8859
+        pydicom.charset.decode(elem, [])
+        assert 'iso8859' in elem.value.encodings
+
 
 if __name__ == "__main__":
     # This is called if run alone, but not if loaded through run_tests.py
diff --git a/pydicom/tests/test_uid.py b/pydicom/tests/test_uid.py
index f035c4cdd..2d7938b89 100644
--- a/pydicom/tests/test_uid.py
+++ b/pydicom/tests/test_uid.py
@@ -3,7 +3,7 @@
 
 import pytest
 
-from pydicom.uid import UID, generate_uid, PYDICOM_ROOT_UID
+from pydicom.uid import UID, generate_uid, PYDICOM_ROOT_UID, JPEGLSLossy
 
 
 class TestGenerateUID(object):
@@ -61,7 +61,6 @@ class TestUID(object):
         assert not self.uid == 'Explicit VR Little Endian'
         # Issue 96
         assert not self.uid == 3
-        assert not self.uid is None
 
     def test_inequality(self):
         """Test that UID.__ne__ works."""
@@ -73,7 +72,6 @@ class TestUID(object):
         assert self.uid != 'Explicit VR Little Endian'
         # Issue 96
         assert self.uid != 3
-        assert self.uid is not None
 
     def test_hash(self):
         """Test that UID.__hash_- works."""
@@ -183,6 +181,21 @@ class TestUID(object):
         assert self.uid.name == 'Implicit VR Little Endian'
         assert UID('1.2.840.10008.5.1.4.1.1.2').name == 'CT Image Storage'
 
+    def test_name_with_equal_hash(self):
+        """Test that UID name works for UID with same hash as predefined UID.
+        """
+        class MockedUID(UID):
+            # Force the UID to return the same hash as one of the
+            # uid dictionary entries (any will work).
+            # The resulting hash collision forces the usage of the `eq`
+            # operator while checking for containment in the uid dictionary
+            # (regression test for issue #499)
+            def __hash__(self):
+                return hash(JPEGLSLossy)
+
+        uid = MockedUID('1.2.3')
+        assert uid.name == '1.2.3'
+
     def test_type(self):
         """Test that UID.type works."""
         assert self.uid.type == 'Transfer Syntax'
diff --git a/pydicom/tests/test_valuerep.py b/pydicom/tests/test_valuerep.py
index cd4e0a7fb..49ff72625 100644
--- a/pydicom/tests/test_valuerep.py
+++ b/pydicom/tests/test_valuerep.py
@@ -310,6 +310,15 @@ class PersonNametests(unittest.TestCase):
         msg = "PersonName3 not equal comparison did not work correctly"
         self.assertFalse(pn != "John^Doe", msg)
 
+    def test_encoding_carried(self):
+        """Test encoding is carried over to a new PN3 object"""
+        # Issue 466
+        from pydicom.valuerep import PersonName3
+        pn = PersonName3("John^Doe", encodings='iso_ir_126')
+        assert pn.encodings == ['iso_ir_126', 'iso_ir_126', 'iso_ir_126']
+        pn2 = PersonName3(pn)
+        assert pn2.encodings == ['iso_ir_126', 'iso_ir_126', 'iso_ir_126']
+
 
 class DateTimeTests(unittest.TestCase):
     """Unit tests for DA, DT, TM conversion to datetime objects"""