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

Jump to bottom

make inspect Signature hashable #64533

Closed
1st1 opened this issue Jan 21, 2014 · 12 comments
Closed

make inspect Signature hashable #64533

1st1 opened this issue Jan 21, 2014 · 12 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@1st1
Copy link
Member

1st1 commented Jan 21, 2014

BPO 20334
Nosy @brettcannon, @ncoghlan, @larryhastings, @1st1, @anntzer
Files
  • hashable_signature_01.patch
  • issue20334-2.01.patch
  • signature-hash-and-equality.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 = None
closed_at = <Date 2014-04-08.15:47:22.795>
created_at = <Date 2014-01-21.16:50:24.937>
labels = ['type-feature', 'library']
title = 'make inspect Signature hashable'
updated_at = <Date 2014-09-12.19:50:37.497>
user = 'https://github.com/1st1'

    bugs.python.org fields:

    activity = <Date 2014-09-12.19:50:37.497>
actor = 'yselivanov'
assignee = 'none'
closed = True
closed_date = <Date 2014-04-08.15:47:22.795>
closer = 'yselivanov'
components = ['Library (Lib)']
creation = <Date 2014-01-21.16:50:24.937>
creator = 'yselivanov'
dependencies = []
files = ['33595', '36607', '36610']
hgrepos = []
issue_num = 20334
keywords = ['patch']
message_count = 12.0
messages = ['208671', '208673', '208674', '214962', '215768', '225343', '225361', '225364', '226820', '226831', '226840', '226841']
nosy_count = 6.0
nosy_names = ['brett.cannon', 'ncoghlan', 'larry', 'python-dev', 'yselivanov', 'Antony.Lee']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue20334'
versions = ['Python 3.5']
    @1st1
    Copy link
    Member Author

    1st1 commented Jan 21, 2014

    inspect.Signature and inspect.Parameter are immutable structures, and it makes sense to make them hashable too.

    Patch is attached.

    @1st1 1st1 added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Jan 21, 2014
    @brettcannon
    Copy link
    Member

    This is a new feature so it can't go into Python 3.4.

    @1st1
    Copy link
    Member Author

    1st1 commented Jan 21, 2014

    Fair enough.

    @1st1
    Copy link
    Member Author

    1st1 commented Mar 27, 2014

    If nobody has any objections on this, I'm going to commit this in 3.5 soon.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 8, 2014

    New changeset 932d69ef0c63 by Yury Selivanov in branch 'default':
    inspect: Make Signature and Parameter hashable. Issue bpo-20334.
    http://hg.python.org/cpython/rev/932d69ef0c63

    @1st1 1st1 closed this as completed Apr 8, 2014
    @anntzer
    Copy link
    Mannequin

    anntzer mannequin commented Aug 15, 2014

    The hash function of the Signature class is actually incompatible with the definition of Signature equality, which doesn't consider the order of keyword-only arguments:

    >>> from inspect import signature
>>> s1 = signature(lambda *, x, y: None); s2 = signature(lambda *, y, x: None)
>>> s1 == s2
True
>>> hash(s1) == hash(s2)
False

    Actually the implementation of Signature.__eq__ seems way too complicated; I would suggest making a helper method returning (return_annotation, tuple(non-kw-only-params), frozenset(kw-only-params)) so that __eq__ can compare these values while __hash__ can hash that tuple.

    @1st1
    Copy link
    Member Author

    1st1 commented Aug 15, 2014

    Thanks, Antony, this is a good catch. Your suggestion seems like a good idea. I'll look into this more closely soon.

    @anntzer
    Copy link
    Mannequin

    anntzer mannequin commented Aug 15, 2014

    Actually, that specific solution (using a helper method) will fail because there may be unhashable params (due to unhashable default values or annotations) among the keyword-only arguments, so it may not be possible to build a frozenset (rather, one should compare the {param.name: param if param.kind == KEYWORD_ONLY} dict). The frozenset approach still works for computing the hash as this requires all params to be hashable anyways.

    @1st1
    Copy link
    Member Author

    1st1 commented Sep 12, 2014

    Antonie, I'm attaching a patch (bpo-20334-2.01.patch) to this issue which should fix the problem. Please review.

    @anntzer
    Copy link
    Mannequin

    anntzer mannequin commented Sep 12, 2014

    While your patch works, I think it is a good opportunity to simplify the implementation of Signature.__eq__, which is *much* more complicated than what it should be.
    Please comment on the attached patch, which uses the helper method approach I suggested.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 12, 2014

    New changeset 3b974b61e74d by Yury Selivanov in branch 'default':
    inspect.Signature: Fix discrepancy between __eq__ and __hash__.
    http://hg.python.org/cpython/rev/3b974b61e74d

    @1st1
    Copy link
    Member Author

    1st1 commented Sep 12, 2014

    Antony, I've tweaked the patch a bit and it's now in default branch. Thank you!

    @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-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @brettcannon @1st1