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

Fix __ne__ error, and add opinionated change to __eq__, in BasePoseMatrix #48

Merged
merged 3 commits into from
Jan 31, 2022

Conversation

btalb
Copy link
Contributor

@btalb btalb commented Jan 30, 2022

This PR has two parts.

1. Error in != comparisons
Comparing two of most things using the != operator currently fails with the following error unless you make them lists:

In [6]: SO3() != SO3()   
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-6-16fbcbbd9d14> in <module>
----> 1 SO3() != SO3()

~/qut/repos/spatialmath-python/spatialmath/baseposematrix.py in __ne__(left, right)
   1560 
   1561         """
-> 1562         return [not x for x in left == right]
   1563 
   1564     def _op2(

TypeError: 'bool' object is not iterable

This is due to __ne__ assuming comparison results are a list. Commit 13d6ee1 addresses this.

2. An opinionated change to == comparisons
Currently a runtime assertion error is thrown if two things are compared of different types.

I know I would find simply receive a False return easier to work with in code, as two different types definitely aren't equal. But that's just one opinion, and I can see how the way it currently is also makes sense.

Happy for this part of the PR to be reverted if desired:

git revert 7297788

@petercorke
Copy link
Collaborator

There is precedent for your answer

>>> class C:
...   pass
>>> c=C()
>>> c == 3
False
>>> 

dissimilar objects doesn't raise an exception.

@petercorke petercorke merged commit 135aab3 into bdaiinstitute:master Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants