Issue 1487 - Suggested Fix - Return false when comparing Object against values that can't be unpacked #1488
Merged
sgillies merged 2 commits intoToblerity:mainfrom Feb 20, 2025
Merged
Conversation
added 2 commits
February 20, 2025 15:39
… for situations throwing TypeErrors
…e explicitly checked for equality, otherwise assume not equal
sgillies
requested changes
Feb 20, 2025
Member
sgillies
left a comment
There was a problem hiding this comment.
Thank you @agware ! This is right on the mark and very well done.
sgillies
approved these changes
Feb 20, 2025
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
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.
This is a suggested fix for Issue 1847
In cases where eq is used for an object that cannot be unpacked (i.e. is not an instance of Mapping) simply default to returning false.
Depending on style/preference, another way this could be implemented is:
I went with the Mapping approach as it seemed to tie in more nicely with the inheritance from MutableMapping for Object, but both are pythonic and I'm happy to switch the fix over if you have a preference.
I made the assumption that the decision to do equality explicitly against the unpacked versions of the objects was in part to maintain backwards compatibility with old style Fiona representations of geometries (or similar). So I tried to stay away from anything that would create more direct comparisons between object types (e.g. return false if it's not a dict/Object), leaving the option open for other Mapping based objects to also resolve to True when compared.