Skip to content

v.class: Added test cases #6071

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

Merged
merged 8 commits into from
Jul 25, 2025
Merged

v.class: Added test cases #6071

merged 8 commits into from
Jul 25, 2025

Conversation

dvrohan
Copy link
Contributor

@dvrohan dvrohan commented Jul 16, 2025

No description provided.

@github-actions github-actions bot added vector Related to vector data processing Python Related code is in Python module tests Related to Test Suite labels Jul 16, 2025
echoix
echoix previously requested changes Jul 17, 2025
@echoix echoix dismissed their stale review July 18, 2025 19:40

File renamed

echoix
echoix previously approved these changes Jul 21, 2025
Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

Can you please test the specific break values, not just the number of classes?

@dvrohan
Copy link
Contributor Author

dvrohan commented Jul 21, 2025

I added the breakpoint values for a few tests. Is that what you meant? For a few tests it is not possible to know the values beforehand.

Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

The tests are slow. There are couple options to optimize this. If I understand this correctly the fixture runs for each test, consider running it just once if appropriate. Also running db.execute in loop is expensive, see the manual page of db.execute to avoid that.

@dvrohan
Copy link
Contributor Author

dvrohan commented Jul 23, 2025

I have used a single db.execute query instead of inside a loop. I am also using a module scope for shared state. Please let me know if this works.

@petrasovaa
Copy link
Contributor

I have used a single db.execute query instead of inside a loop. I am also using a module scope for shared state. Please let me know if this works.

It's still the longest pytest (see the windows CI):

============================== slowest durations ==============================
14.75s call     vector/v.class/tests/test_v_class.py::test_v_class_large_dataset
8.55s call     scripts/v.dissolve/tests/v_dissolve_aggregate_test.py::test_aggregate_two_columns
6.20s call     scripts/v.dissolve/tests/v_dissolve_aggregate_test.py::test_aggregate_column_result[univar]
6.18s call     scripts/v.dissolve/tests/v_dissolve_aggregate_test.py::test_aggregate_column_result[None]
5.95s call     scripts/v.dissolve/tests/v_dissolve_aggregate_test.py::test_aggregate_methods[aggregate_methods4]
5.85s call     scripts/v.dissolve/tests/v_dissolve_aggregate_test.py::test_aggregate_methods[aggregate_methods0]
5.84s call     scripts/v.dissolve/tests/v_dissolve_aggregate_test.py::test_aggregate_methods[aggregate_methods3]
5.77s call     scripts/v.dissolve/tests/v_dissolve_aggregate_test.py::test_aggregate_methods[aggregate_methods2]
5.38s setup    vector/v.fill.holes/tests/v_fill_holes_test.py::test_removal

Perhaps mark the large data test as skipped and see if it helps.

@dvrohan
Copy link
Contributor Author

dvrohan commented Jul 24, 2025

I have reduced the number of points to 100 instead of 1000. I think that should reduce the processing time significantly.

Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

It looks good now, thanks!

@petrasovaa petrasovaa enabled auto-merge (squash) July 25, 2025 11:42
@petrasovaa petrasovaa merged commit d3eef27 into OSGeo:main Jul 25, 2025
30 of 32 checks passed
@github-actions github-actions bot added this to the 8.5.0 milestone Jul 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module Python Related code is in Python tests Related to Test Suite vector Related to vector data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants