Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

The Mapping ABC's __eq__ method should return NotImplemented if the other type is not a Mapping #52975

Closed
stutzbach mannequin opened this issue May 15, 2010 · 10 comments
Closed

The Mapping ABC's __eq__ method should return NotImplemented if the other type is not a Mapping #52975

stutzbach mannequin opened this issue May 15, 2010 · 10 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@stutzbach
Copy link
Mannequin

stutzbach mannequin commented May 15, 2010

BPO 8729
Nosy @rhettinger, @pitrou, @benjaminp
Files
  • mapping.patch: Change Mapping.eq to return NotImplemented if the other type isn't a Mapping
  • eq-test.patch: Add test cases
  • mapping2.patch: Revised patch, with fix and unit tests
    		•

    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 = None
closed_at = <Date 2010-05-21.20:52:37.002>
created_at = <Date 2010-05-15.22:08:11.016>
labels = ['type-bug', 'library']
title = "The Mapping ABC's __eq__ method should return NotImplemented if the other type is not a Mapping"
updated_at = <Date 2010-05-21.20:52:37.001>
user = 'https://bugs.python.org/stutzbach'

    bugs.python.org fields:

    activity = <Date 2010-05-21.20:52:37.001>
actor = 'benjamin.peterson'
assignee = 'none'
closed = True
closed_date = <Date 2010-05-21.20:52:37.002>
closer = 'benjamin.peterson'
components = ['Library (Lib)']
creation = <Date 2010-05-15.22:08:11.016>
creator = 'stutzbach'
dependencies = []
files = ['17355', '17392', '17433']
hgrepos = []
issue_num = 8729
keywords = ['patch']
message_count = 10.0
messages = ['105833', '105846', '105849', '106000', '106001', '106017', '106107', '106131', '106257', '106264']
nosy_count = 4.0
nosy_names = ['rhettinger', 'pitrou', 'benjamin.peterson', 'stutzbach']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue8729'
versions = ['Python 2.6', 'Python 3.1', 'Python 2.7', 'Python 3.2']
    @stutzbach
    Copy link
    Mannequin Author

    stutzbach mannequin commented May 15, 2010

    The Mapping ABC's __eq__ method should return NotImplemented if the other type is not a Mapping, to give the other type a chance at the comparison. Right now it simply returns false.

    The comparison methods on the other ABCs in _abcoll.py already return NotImplemented if they don't recognize the other type.

    @stutzbach stutzbach mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels May 15, 2010
    @benjaminp
    Copy link
    Contributor

    A test would be good.

    @stutzbach
    Copy link
    Mannequin Author

    stutzbach mannequin commented May 16, 2010

    Will do, sometime this week.

    @rhettinger
    Copy link
    Contributor

    Backport to 2.6 and 3.1?

    @pitrou
    Copy link
    Member

    pitrou commented May 18, 2010

    Yes, I think this is a good candidate for backport. The ABCs are new and their APIs shouldn't contain any obvious bugs such as this.

    @stutzbach
    Copy link
    Mannequin Author

    stutzbach mannequin commented May 19, 2010

    Here's a patch that adds test cases. It exercises all of the following special methods on Set and Mapping to ensure that they return NotImplemented if they don't recognize the other type.

    lt, gt, le, ge, eq, ne, or, and, xor, sub

    I made the patch against the py3k branch.

    I made the test-case patch separate to make it easier to see the before and after behavior of the actual fix.

    @benjaminp
    Copy link
    Contributor

    Will you post to Rietveld, please?

    @stutzbach
    Copy link
    Mannequin Author

    stutzbach mannequin commented May 20, 2010

    Done: http://codereview.appspot.com/1193044

    This is my first time using Rietveld. Let me know if I've done anything wrong.

    @stutzbach
    Copy link
    Mannequin Author

    stutzbach mannequin commented May 21, 2010

    Here is a revised patch based on Benjamin's comments on Rietveld.

    @benjaminp
    Copy link
    Contributor

    Fixed in r81414. Thanks for the patch.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @benjaminp @rhettinger @pitrou