Issue26711
Created on 2016-04-07 20:06 by serhiy.storchaka, last changed 2016-05-01 10:46 by serhiy.storchaka. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| plistlib_data_eq.patch | serhiy.storchaka, 2016-04-07 20:06 | review | ||
| plistlib_data_eq_2.patch | serhiy.storchaka, 2016-04-08 11:44 | review | ||
| Messages (7) | |||
|---|---|---|---|
| msg263001 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2016-04-07 20:06 | |
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__(). |
|||
| msg263018 - (view) | Author: Ronald Oussoren (ronaldoussoren) * | Date: 2016-04-08 10:56 | |
I don't understand the explicit use of __eq__ in the bytes case. Why is that needed? |
|||
| msg263020 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2016-04-08 11:24 | |
>>> 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 "==". |
|||
| msg264567 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2016-04-30 17:21 | |
Ronald, my second patch uses "==" instead of __eq__. Is it good to you? |
|||
| msg264589 - (view) | Author: Ronald Oussoren (ronaldoussoren) * | Date: 2016-05-01 08:51 | |
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 |
|||
| msg264592 - (view) | Author: Roundup Robot (python-dev) | Date: 2016-05-01 10:37 | |
New changeset 41afb83cffac by Serhiy Storchaka in branch '3.5': Issue #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 #26711: Fixed the comparison of plistlib.Data with other types. https://hg.python.org/cpython/rev/dbdd5bc4df99 |
|||
| msg264593 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * | Date: 2016-05-01 10:46 | |
For simplicity I left "==" and fixed only obvious bug. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2016-05-01 10:46:01 | serhiy.storchaka | set | status: open -> closed messages: + msg264593 assignee: serhiy.storchaka |
| 2016-05-01 10:37:09 | python-dev | set | nosy:
+ python-dev messages: + msg264592 |
| 2016-05-01 08:51:28 | ronaldoussoren | set | messages: + msg264589 |
| 2016-04-30 17:21:03 | serhiy.storchaka | set | messages: + msg264567 |
| 2016-04-08 11:44:01 | serhiy.storchaka | set | files: + plistlib_data_eq_2.patch |
| 2016-04-08 11:24:19 | serhiy.storchaka | set | messages: + msg263020 |
| 2016-04-08 10:56:56 | ronaldoussoren | set | messages: + msg263018 |
| 2016-04-07 20:06:42 | serhiy.storchaka | create | |