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

[Mapping] Implementing == and != operators #12317

Merged
merged 3 commits into from
May 3, 2024

Conversation

roigcarlo
Copy link
Member

📝 Description
== and != operators are needed for the GTest

currently the tests assume that distance is not checked, but it should?

🆕 Changelog

  • Added == operator
  • Added != operator
  • Modify the tests accordingly

@roigcarlo roigcarlo requested a review from philbucher April 26, 2024 09:16
@roigcarlo roigcarlo marked this pull request as ready for review April 26, 2024 09:17
@roigcarlo roigcarlo requested a review from a team as a code owner April 26, 2024 09:17
@roigcarlo roigcarlo mentioned this pull request Apr 26, 2024
6 tasks
loumalouomega
loumalouomega previously approved these changes Apr 26, 2024
@roigcarlo
Copy link
Member Author

@loumalouomega I am not sure if we can approve this right away, the idea was to discuss with @philbucher if the equality operators need to take into consideration the distance or not.

@philbucher
Copy link
Member

I am aware, will check in on the weekend. This is sth I need to think about and remember what I did
Otherwise it will break the braycentric mapper

@roigcarlo
Copy link
Member Author

Any news?

philbucher
philbucher previously approved these changes Apr 30, 2024
Copy link
Member

@philbucher philbucher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I remember!
The id does not matter for the comparison, I only care about the distance
I.e. the inheritance from IndexedObject does not matter here

@philbucher
Copy link
Member

wait, I think there is more to it after all 🤔

@roigcarlo
Copy link
Member Author

@philbucher Done with the changes.

@roigcarlo roigcarlo enabled auto-merge May 3, 2024 12:54
@roigcarlo roigcarlo merged commit f9b960a into master May 3, 2024
11 checks passed
@roigcarlo roigcarlo deleted the mapping/close-points-operators branch May 3, 2024 14:14
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.

3 participants