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.

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

datetime's comparison methods do not return NotImplemented when they should #52253

Closed
wsanchez mannequin opened this issue Feb 23, 2010 · 15 comments
Closed

datetime's comparison methods do not return NotImplemented when they should #52253

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

Comments

@wsanchez
Copy link
Mannequin

wsanchez mannequin commented Feb 23, 2010

BPO 8005
Nosy @tim-one, @birkenfeld, @mdickinson, @abalkin, @bitdancer
Files
  • datetime_foreigncompare_test.patch: test for comparing datetime objects against foreign objects
  • 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-07-31.18:27:01.427>
    created_at = <Date 2010-02-23.21:27:36.151>
    labels = ['type-bug', 'library']
    title = "datetime's comparison methods do not return NotImplemented when they should"
    updated_at = <Date 2010-07-31.18:27:01.425>
    user = 'https://bugs.python.org/wsanchez'

    bugs.python.org fields:

    activity = <Date 2010-07-31.18:27:01.425>
    actor = 'georg.brandl'
    assignee = 'stutzbach'
    closed = True
    closed_date = <Date 2010-07-31.18:27:01.427>
    closer = 'georg.brandl'
    components = ['Library (Lib)']
    creation = <Date 2010-02-23.21:27:36.151>
    creator = 'wsanchez'
    dependencies = []
    files = ['16408']
    hgrepos = []
    issue_num = 8005
    keywords = []
    message_count = 15.0
    messages = ['99954', '99984', '100269', '104291', '104305', '104327', '104328', '104329', '104331', '104332', '104333', '104345', '106499', '106500', '112180']
    nosy_count = 8.0
    nosy_names = ['tim.peters', 'georg.brandl', 'mark.dickinson', 'belopolsky', 'wsanchez', 'stutzbach', 'r.david.murray', 'twb']
    pr_nums = []
    priority = 'normal'
    resolution = 'wont fix'
    stage = 'needs patch'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue8005'
    versions = ['Python 2.6', 'Python 2.7']

    @wsanchez
    Copy link
    Mannequin Author

    wsanchez mannequin commented Feb 23, 2010

    datetime's comparison methods do not return NotImplemented when they should. As a result, if you implement a class that can compare itself with a datetime object, this only works if your class on on the left side of the comparison operation.

    This is true for equality as well as ordering operations.

    @wsanchez wsanchez mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Feb 23, 2010
    @bitdancer
    Copy link
    Member

    Confirmed on trunk, works as expected in py3k.

    @twb
    Copy link
    Mannequin

    twb mannequin commented Mar 1, 2010

    Work-in-progress test for this issue. I'll clean up the test and start work on a patch later tonight.

    @stutzbach
    Copy link
    Mannequin

    stutzbach mannequin commented Apr 26, 2010

    Thomas, could you write a unit test to go along with your patch?

    @mdickinson
    Copy link
    Member

    See bpo-5516 for a related issue.

    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Apr 27, 2010

    Is there a patch with a fix or just a patch with a test. If the later, maybe someone can remove a patch keyword.

    @twb
    Copy link
    Mannequin

    twb mannequin commented Apr 27, 2010

    It's just a test. Finishing the patch completely slipped my mind. I'll work on it later tonight.

    @stutzbach
    Copy link
    Mannequin

    stutzbach mannequin commented Apr 27, 2010

    You're right. I asked Thomas the wrong question, and the Keywords and Stage need updating.

    Thomas, do you still plan to submit a patch that fixes the problem?

    @twb
    Copy link
    Mannequin

    twb mannequin commented Apr 27, 2010

    I'm still reasonably new to the codebase, but I'm certainly going to try to fix the issue.

    @stutzbach
    Copy link
    Mannequin

    stutzbach mannequin commented Apr 27, 2010

    Great! If you get stuck or have a question, just ask. :-)

    @stutzbach stutzbach mannequin self-assigned this Apr 27, 2010
    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Apr 27, 2010

    Before someone spends more time writing a patch, lets pause and consider whether this is a bug in the first place.

    My understanding of the issue is that given

    >>> class A(object):
    ...   def __eq__(self, other):
    ...    return True
    ... 

    OP expects date(1,1,1) == A() to return True, but instead

    >>> date(1,1,1) == A()
    False

    Note that this problem can be worked around by endowing A with a timetuple attribute:

    >> A.timetuple=1

    Now

    >>> date(1,1,1) == A()
    True

    This is a documented feature:

    """
    In order to stop comparison from falling back to the default scheme of comparing object addresses, date comparison normally raises TypeError if the other comparand isn’t also a date object. However, NotImplemented is returned instead if the other comparand has a timetuple() attribute. This hook gives other kinds of date objects a chance at implementing mixed-type comparison. If not, when a date object is compared to an object of a different type, TypeError is raised unless the comparison is == or !=. The latter cases return False or True, respectively.
    """ http://docs.python.org/release/2.6.5/library/datetime.html#datetime.date.day (Note 4)

    I am adding Tim to the "nosy" list because he appears to be the author of the relevant code.

    It is my understanding that this issue can only be regarded as an RFE. Given the fact that 2.x is approaching bug fix only stage (if it is not already there) and the problem is fixed in 3.x, I recommend closing this as "won't fix."

    Mark,

    No, I don't think this is directly related to bpo-5516.

    @stutzbach
    Copy link
    Mannequin

    stutzbach mannequin commented Apr 27, 2010

    Thanks for pointing that out.

    For what it's worth, if I understand the documentation correctly the goal is to prevent the following misleading comparisons:

    date with time
    datetime with date
    datetime with time
    datetime w/ timezone with datetime w/o timezone
    time w/ timezone with time w/o timezone

    It's unfortunate that it throws a TypeError for all comparisons (unlike complex numbers which only throw a TypeError when comparing with numbers), but I suppose you are right that it's too late to fix this in 2.x.

    @abalkin
    Copy link
    Member

    abalkin commented May 26, 2010

    Any objections to closing this as "won't fix"?

    @stutzbach
    Copy link
    Mannequin

    stutzbach mannequin commented May 26, 2010

    None here.

    @birkenfeld
    Copy link
    Member

    Closing now.

    @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
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants