Python3: Provide __hash__ methods where ever we have provided __eq__ methods#9757
Merged
Conversation
Collaborator
|
Regarding the location helper classes, I assume hash is defined on tuple as well. Why can't we just call super or |
Member
Author
|
Oh, I didn't see that they inherited from NamedTuple. In that case we
can just call super :)
|
… are NamedTuples.
Member
Author
|
@LeonarddeR I have addressed your comment now. |
LeonarddeR
approved these changes
Jun 17, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
None.
Summary of the issue:
Replacement for pr #9746 as it was reverted with pr #9756.
Not all cases were handled with that approach.
In baseObject.AutoPropertyObject: all instances are tracked on the class by storing them in a WeakKeyDictionary. Tracking of these instances is to allow clearing the proprty cache of all living objects in one go.
In eventHandler: there are several dictionaries that track currently queued events, keyed by eventName, object, or eventName and object.
In browseMode: _isNVDAObject in application keeps a cache of results, keyed by NvDAObject.
Storing NvDAObjects and or TextInfo objects in these dictionaries requires these objects to be hashable.
However, in Python3, if an object's equality check is customized (I.e. a custom eq method is provided), the object becomes unhashable, as Python3 requires that any two objects that report being equal must also share the same hash value.
If eq is implemented on an object and there is a need for the object to be hashable, one must also implement hash on the object.
Description of how this pull request fixes the issue:
Provides a
__hash__methods on all classes we have provided a__eq__method.In the majority of cases, the implementation just calls super, which falls back to the default hash implementation which uses the object's address for the hash. However, for the cases in locationHelper, the hash is generated from the member variables (I.e. point's hash is a hash of the tuple holding self.x and self.y). This makes for a more optimal hash for storing collections of numbers like this.
Testing performed:
Again, no TypeError("unhashable type") is raised when starting NvDA dn or using broaseMode documents.
Known issues with pull request:
None.
Change log entry:
None.
Section: New features, Changes, Bug fixes