Fix comparison of plistlib.Data

[serhiy-storchaka]

BPO	26711
Nosy	@ronaldoussoren, @serhiy-storchaka
Files	plistlib_data_eq.patch plistlib_data_eq_2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2016-05-01.10:46:01.315>
created_at = <Date 2016-04-07.20:06:42.242>
labels = ['type-bug', 'library']
title = 'Fix comparison of plistlib.Data'
updated_at = <Date 2016-05-01.10:46:01.314>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2016-05-01.10:46:01.314>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2016-05-01.10:46:01.315>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2016-04-07.20:06:42.242>
creator = 'serhiy.storchaka'
dependencies = []
files = ['42394', '42398']
hgrepos = []
issue_num = 26711
keywords = ['patch']
message_count = 7.0
messages = ['263001', '263018', '263020', '264567', '264589', '264592', '264593']
nosy_count = 3.0
nosy_names = ['ronaldoussoren', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26711'
versions = ['Python 3.5', 'Python 3.6']

[serhiy-storchaka]

Proposed patch fixes several bugs in plistlib.Data.__eq__().

• isinstance(other, str) was used instead of isinstance(other, bytes). Data always wraps bytes and should be comparable with bytes. str was correct type in Python 2.

• id(self) == id(other) is always false, because if other is self, the first condition (isinstance(other, self.__class__)) should be true. NotImplemented should be returned as fallback. This allows comparing with Data subclasses and correct work of __ne__().

• The __eq__() method should be used instead of the equality operator. This is needed for correct work in case if value is bytes subclass with overloaded __eq__().

[ronaldoussoren]

I don't understand the explicit use of __eq__ in the bytes case. Why is that needed?

[serhiy-storchaka]

>>> import plistlib
>>> class MyData(bytes):
...     def __eq__(self, other):
...         if isinstance(other, plistlib.Data):
...             return super().__eq__(other.value)
...         return False
... 
>>> plistlib.Data(b'x') == MyData(b'x')
True

If use the equality operator the result is False.

I don't know if this is good example. In any case this is corner case and we can manage with "==".

[serhiy-storchaka]

Ronald, my second patch uses "==" instead of __eq__. Is it good to you?

[ronaldoussoren]

Serhiy,

I slightly prefer the second patch, but either one would be fine.

The reason I asked about explicitly calling __eq__ is that this is an uncommon pattern (at least for me).

Ronald

[python-dev]

New changeset 41afb83cffac by Serhiy Storchaka in branch '3.5':
Issue bpo-26711: Fixed the comparison of plistlib.Data with other types.
https://hg.python.org/cpython/rev/41afb83cffac

New changeset dbdd5bc4df99 by Serhiy Storchaka in branch 'default':
Issue bpo-26711: Fixed the comparison of plistlib.Data with other types.
https://hg.python.org/cpython/rev/dbdd5bc4df99

[serhiy-storchaka]

For simplicity I left "==" and fixed only obvious bug.