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

[strategies] Fix spherical closest_points strategy for PointOfSegment different than model::point #1271

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

awulkiew
Copy link
Member

@awulkiew awulkiew commented Apr 5, 2024

Currently it fails to compile due to multiple return types.

Copy link
Member

@vissarion vissarion left a comment

Choose a reason for hiding this comment

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

thanks!

Copy link
Collaborator

@barendgehrels barendgehrels left a comment

Choose a reason for hiding this comment

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

Thanks
(we could add a test for this situation)

@awulkiew
Copy link
Member Author

Thanks (we could add a test for this situation)

@barendgehrels would you prefer me to create a separate test case for user-defined types or maybe to replace BG models in all of them? If it's the latter then maybe we should use types different than BG models in all of the tests?

@barendgehrels
Copy link
Collaborator

Thanks (we could add a test for this situation)

@barendgehrels would you prefer me to create a separate test case for user-defined types or maybe to replace BG models in all of them? If it's the latter then maybe we should use types different than BG models in all of the tests?

It would indeed be good that every algorithm also contains one test for one different type. To test type diversity. The algorithms themselves can be tested with the standard type.

So in this case I would suggest to create a separate test case for user-defined types.

There is often also constness etc involved (I'm not asking that right now). Many of our test (not generating points) will not compile, probably. And algorithms working on two or three input/output's might contain a test case where different types (or even coordinate types) are involved. But most tests don't do this.

I made this remark because there was a particular bug fixed related to this - then it would be good to add such a case (if you have the time).

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

3 participants