Repository: pydicom/pydicom

Issue:
Dataset equality fails when elements added in different order
#### Description
Dataset equality produces incorrect results

#### Steps/Code to Reproduce
```python
>>> from pydicom.dataset import Dataset
>>> ds = Dataset()
>>> ds.PatientName = 'Test'
>>> ds.PatientID = '1234'
>>> ds2 = Dataset()
>>> ds2.PatientID = '1234'
>>> ds2.PatientName = 'Test
>>> ds == ds2
False
>>> ds2 = Dataset()
>>> ds2.PatientName = 'Test'
>>> ds2.PatientID = '1234'
>>> ds == ds2
True
```

#### Versions
```python
>>> import platform; print(platform.platform())
Linux-4.10.0-30-generic-x86_64-with-Ubuntu-17.04-zesty
>>> import sys; print("Python", sys.version)
Python 3.6.1 (default, Mar 22 2017, 06:17:05) 
[GCC 6.3.0 20170321]
>>> import pydicom; print("pydicom", pydicom.__version__)
pydicom 1.0.0a1
```

Discussion:
darcymason: Just noticed that since only values are compared, it is possible to pass even if some of the tags are different but have same value (very unlikely, but could happen if many entries were empty string, for example).

Would be nice to add a test too, e.g. the example from the issue first comment.

I'm not sure deriving from OrderedDict is worth it, there are only a few places where ordering is needed.
mrbean-bremen: Ok, you are probably right - it doesn't make much sense then.
scaramallion: `Dataset.values()` returns a list of `DataElement` so different tags with the same value will return False since `DataElement` equality checks tag, VR and value.

I already added a test reproducing #462 as my first commit, unless you're referring to something else?
darcymason: You're right of course, my bad, just looking too quickly

Patch:
diff --git a/pydicom/dataset.py b/pydicom/dataset.py
index 69b68f7ef..6e0f82daf 100644
--- a/pydicom/dataset.py
+++ b/pydicom/dataset.py
@@ -400,8 +400,10 @@ class Dataset(dict):
             # __dict__
             # Convert values() to a list for compatibility between
             #   python 2 and 3
-            return (list(self.values()) == list(other.values()) and
-                    self.__dict__ == other.__dict__)
+            # 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
 


Test patch:
diff --git a/pydicom/tests/test_dataset.py b/pydicom/tests/test_dataset.py
index 01dfa0453..f25160446 100644
--- a/pydicom/tests/test_dataset.py
+++ b/pydicom/tests/test_dataset.py
@@ -401,9 +401,11 @@ class DatasetTests(unittest.TestCase):
         """Dataset: equality returns correct value with simple dataset"""
         d = Dataset()
         d.SOPInstanceUID = '1.2.3.4'
+        d.PatientName = 'Test'
         self.assertTrue(d == d)
 
         e = Dataset()
+        e.PatientName = 'Test'
         e.SOPInstanceUID = '1.2.3.4'
         self.assertTrue(d == e)
 
@@ -457,6 +459,7 @@ class DatasetTests(unittest.TestCase):
         d.SOPInstanceUID = '1.2.3.4'
         d.BeamSequence = []
         beam_seq = Dataset()
+        beam_seq.PatientID = '1234'
         beam_seq.PatientName = 'ANON'
         d.BeamSequence.append(beam_seq)
         self.assertTrue(d == d)
@@ -466,6 +469,7 @@ class DatasetTests(unittest.TestCase):
         e.BeamSequence = []
         beam_seq = Dataset()
         beam_seq.PatientName = 'ANON'
+        beam_seq.PatientID = '1234'
         e.BeamSequence.append(beam_seq)
         self.assertTrue(d == e)