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.

Sign up for GitHub

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

Binary Predicate Test Dispatching #1085

Merged
merged 9 commits into from
Apr 26, 2023
Merged

Binary Predicate Test Dispatching #1085

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
86ad906
First commit of test dispatch branch.
thomcom Apr 19, 2023
1a0072c
Enable xfail.
thomcom Apr 19, 2023
b2ea930
Binpred test dispatch documented and ready for review.
thomcom Apr 19, 2023
dfcd583
Fix bug in test list.
thomcom Apr 19, 2023
1aefaa2
Update python/cuspatial/cuspatial/tests/binpreds/test_binpred_test_di…
thomcom Apr 20, 2023
b636a71
Merge branch 'branch-23.06' into feature/test-dispatch
thomcom Apr 20, 2023
58982db
Merge branch 'branch-23.06' into feature/test-dispatch
thomcom Apr 26, 2023
c9e3bf3
Add description to why every test returns a GeoSeries of length 3.
thomcom Apr 26, 2023
2922d2c
Merge branch 'branch-23.06' into feature/test-dispatch
thomcom Apr 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 37 additions & 1 deletion python/cuspatial/cuspatial/tests/binpreds/binpred_test_dispatch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,43 @@ def object_dispatch(name_list):


Copy link
Member

Choose a reason for hiding this comment

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

Can't you rely on the machinery of @pytest.mark.parameterize() to build up the Cartesian product of test combinations rather than using nested loops?

types = [Point, Linestring, Polygon]
predicates = [disjoint, intersects, overlaps, contains, constains_properly, within, etc.]

@pytest.mark.parametrize('LHS type', types)
@pytest.mark.parametrize('RHS type', types)
@pytest.mark.parametrize('predicate', predicates)
@pytest.mark.parametrize('the data', the_data)
def test(...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started out just parameterizing the cases in the fashion that you describe. Because each of the feature combinations is predefined, I needed to have a separate test with separate parameterizations for exclusive set of features (point, linestring), (point, polygon), etc. Because I wanted logging and reporting on the test results, it was much easier to move that into a single test, and a single list of features that are used in the test. The primary contribution of this PR is the hand-coded list in features.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I remember the specific reason that I parameterize the tests according to the pattern in https://github.com/rapidsai/cuspatial/pull/1085/files#diff-7064c17cb647ac9ab6d1e6f5c4c1726f68fd5b20205dd85f1d34684d53fd3ea6R20-R35 instead of using the pattern that you described. The simple answer is because you cannot parameterize a pytest fixture, only native iterables. I am using the @pytest.mark.parameterize functionality, I've just moved the generation of the tests into the fixture instead of passing a list to the @pytest.mark.parameterize call.

def simple_test_dispatch():
"""Generates a list of test cases for each geometry type combination."""
"""Generates a list of test cases for each geometry type combination.

Each dispatched test case is a tuple of the form:
(test_name, test_description, lhs, rhs)
which is run in `test_binpred_test_dispatch.py`.

The test_name is a unique identifier for the test case.
The test_description is a string representation of the test case.
The lhs and rhs are GeoSeries of the left and right geometries.

lhs and rhs are always constructed as a list of 3 geometries since
the binpred function is designed to operate primarily on groups of
geometries. The first and third feature in the list always match
the first geometry specified in `test_description`, and the rhs is always
a group of three of the second geometry specified in `test_description`.
The second feature in the lhs varies.

When the types of the lhs and rhs are equal, the second geometry
from `test_description` is substituted for the second geometry in the lhs.
This produces a test form of:
lhs rhs
A B
B B
A B
isVoid marked this conversation as resolved.
Show resolved Hide resolved

This decision has two primary benefits:
1. It causes the test to produce varied results (meaning results of the
form (True, False, True) or (False, True, False), greatly reducing the
likelihood of an "all-False" or "all-True" predicate producing
false-positive results.
2. It tests every binary predicate against self, such as A.touches(A)
for every predicate and geometry combination.

When the types of lhs and rhs are not equal this variation is not
performed, since we cannot currently use predicate operations on mixed
geometry types.
"""
for types in type_dispatch:
generator = type_dispatch[types]
for test_name, test_description, lhs, rhs in generator:
Expand Down