Skip to content

Conversation

@kashish2710
Copy link

This PR adds a regression test case for Boost.Geometry issue #1439.

– Introduces case_1439 in overlay_cases.hpp
– The case represents two non-overlapping polygons
– This case is intended to be used by set operation tests

No algorithm behavior is changed; this only adds test data.

@kashish2710
Copy link
Author

This PR adds the overlay regression case for issue #1439.

"POLYGON((0 7,-5 6,11 -13,0 7))"
};

// Issue 1439: non-overlapping polygons reported as full intersection
Copy link
Collaborator

Choose a reason for hiding this comment

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

But it is not called yet? Or do you still have to push something?

Copy link
Author

Choose a reason for hiding this comment

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

You’re right — the case was not called yet.

I’ve now added an overlay intersection test in overlay.cpp for case_1439
using TEST_INTERSECTION, wrapped in
BOOST_GEOMETRY_TEST_ENABLE_FAILING,
and pushed the update to this PR.

Copy link
Author

Choose a reason for hiding this comment

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

Hi! Thanks for the review 👍
Just checking if there’s anything else I should update before this can be merged.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's fine now. I added @vissarion and @tinko92 for short optional additional reviews.
I will look tomorrow at the case itself, and get back to you.

Thanks again for your quick efforts.

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!

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.

I am OK, I left only some minor comments. Thanks @kashish2710 !
Feel free to merge your commits into one. In any case we can still squash merge the PR.

#ifdef BOOST_GEOMETRY_TEST_ENABLE_FAILING
// Issue 1439
// Regression test: intersection of non-overlapping polygons should be empty.
TEST_INTERSECTION(case_1439, 0, 0, 0.0);
Copy link
Member

Choose a reason for hiding this comment

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

maybe issue_1439 is more descriptive


#ifdef BOOST_GEOMETRY_TEST_ENABLE_FAILING
// Issue 1439
// Regression test: intersection of non-overlapping polygons should be empty.
Copy link
Member

Choose a reason for hiding this comment

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

please fix indentation

@kashish2710
Copy link
Author

Thanks @vissarion for the review and suggestions!
I’ve renamed the test to issue_1439, fixed the indentation, and removed the redundant comment.
Appreciate the feedback. 🚀

Copy link
Collaborator

@tinko92 tinko92 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

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.

4 participants