-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
make inspect Signature hashable #64533
Comments
inspect.Signature and inspect.Parameter are immutable structures, and it makes sense to make them hashable too. Patch is attached. |
This is a new feature so it can't go into Python 3.4. |
Fair enough. |
If nobody has any objections on this, I'm going to commit this in 3.5 soon. |
New changeset 932d69ef0c63 by Yury Selivanov in branch 'default': |
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. |
Thanks, Antony, this is a good catch. Your suggestion seems like a good idea. I'll look into this more closely soon. |
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. |
Antonie, I'm attaching a patch (bpo-20334-2.01.patch) to this issue which should fix the problem. Please review. |
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. |
New changeset 3b974b61e74d by Yury Selivanov in branch 'default': |
Antony, I've tweaked the patch a bit and it's now in default branch. Thank you! |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: