UID not equal comparison by description wrong

[GoogleCodeExporter]

What steps will reproduce the problem?
>>> ds=dicom.read_file("CT_small.dcm")

>>> ds.file_meta.MediaStorageSOPClassUID == "CT Image Storage"
True   # as it should be

# Try same comparison with not equal
>>> ds.file_meta.MediaStorageSOPClassUID != "CT Image Storage"
True   # should be False

>>> ds.file_meta.MediaStorageSOPClassUID != '1.2.840.10008.5.1.4.1.1.2'
False  # works correctly when compared to UID 

Original issue reported on code.google.com by darcymason@gmail.com on 28 Jan 2013 at 1:50

[GoogleCodeExporter]

Good catch!

I'm assuming the fix should be something as simple as:

def __ne__(self, obj):
    return not self == obj

More generally, in pydicom there are a number of classes derived from str. It 
would probably be best to go through and find those and make sure that they 
implement the != operator if they overload the == operator.

Original comment by Suever@gmail.com on 28 Jan 2013 at 4:30

[GoogleCodeExporter]

You would think the __ne__ definition should be enough, but I had a vague 
recollection that I read somewhere there is more to it. Plus I had thought in 
the absence of __ne__ that it would automatically call __eq__, but according to 
http://docs.python.org/2/reference/datamodel.html#basic-customization, it does 
not, and __ne__ should be defined any time __eq__ is.

As to all other class, we have to be careful about __hash__ definition -- it 
should refer to the parent __hash__ if an immutable (all ours are, I think); if 
mutable than it should be set to None. 
http://docs.python.org/3/reference/datamodel.html#object.__hash__ and
http://docs.python.org/2/reference/datamodel.html#object.__hash__

So looks like __ne__ is appropriate, but we should have unit tests too, and do 
this for all classes as Jonathan said.

Original comment by darcymason@gmail.com on 28 Jan 2013 at 6:02

[GoogleCodeExporter]

Checked for spots that define a __eq__ or __ne__ across all pydicom code
$> grep -rHin --include "*.py" "__eq__\|__ne__" *
UID.py:99:    def __eq__(self, other):
UID.py:101:        if str.__eq__(self, other) is True:  # 'is True' needed 
(issue 96)
UID.py:103:        if str.__eq__(self.name, other) is True:  # 'is True' needed 
(issue 96)
UID.py:122:    # For python 3, any override of __cmp__ or __eq__ immutable 
requires
tag.py:58:    def __eq__(self, other):
tag.py:67:    def __ne__(self, other):
tag.py:76:    # For python 3, any override of __cmp__ or __eq__ immutable 
requires
valuerep.py:240:    def __eq__(self, other):

So UID and valuerep.py (class PersonName3) appear to be the only places of 
concern for this issue.

Original comment by darcymason@gmail.com on 19 Jul 2013 at 10:56

[GoogleCodeExporter]

This issue was closed by revision 1582bf62a903.

Original comment by darcymason@gmail.com on 19 Jul 2013 at 11:31

• Changed state: Fixed