Open-MSS
Improve Test Coverage and Refactor Test for mslib.utils.coordinate
This PR enhances the test suite for the mslib.utils.coordinate module.
Key Updates:
- Refactored and added new test cases for functions in the coordinate module.
- Used pytest.mark.parametrize to simplify and expand parameterized testing.
- Improved edge case coverage for distance, projection, and angle calculations.
- Enhanced test readability and maintainability with detailed docstrings.
- Validated linear and great circle path generation for lat/lon points.
Purpose of PR?:
Fixes #
Does this PR introduce a breaking change? This PR does not introduce any breaking changes.
If the changes in this PR are manually verified, list down the scenarios covered::
If manually verified, the following scenarios were tested:
- Distance calculations across diverse lat/lon coordinates.
- Angle normalization and edge cases, including negative and large angles.
- Path generation for both linear and great circle connections with varied numpoints.
- Projection parameter retrieval for valid and invalid projection strings.
Additional information for reviewer? : This PR is an independent contribution and does not depend on or continue from previous PRs.
Does this PR results in some Documentation changes? No documentation changes are required.
Checklist:
- [ ] Bug fix. Fixes
- [ ] New feature (Non-API breaking changes that adds functionality)
- [x] PR Title follows the convention of
<type>: <subject> - [x] Commit has unit tests
Hi @saiabhinav001 Maybe a push to the repo is missing? I do see the class names here and also there is a lint failure.
interesting how pytest.approx does this.
FAILED tests/_test_utils/test_coordinate.py::TestAngles::test_rotate_point[point5-45-rotated_point5] - assert (-1.767766952...7669529663689) == approx((-1.76...78 ± 1.8e-06))
comparison failed. Mismatched elements: 2 / 2:
Max absolute difference: 3.3047033631383727e-05
Max relative difference: 1.8694225263081064e-05
Index | Obtained | Expected
0 | -1.7677669529663687 | -1.7678 ± 1.8e-06
1 | 1.7677669529663689 | 1.7678 ± 1.8e-06
====== 1 failed, 588 passed, 13 skipped, 34 warnings in 470.39s (0:07:50) ======
ERROR conda.cli.main_run:execute(124): `conda run xvfb-run pytest -v -n 6 --dist loadfile --max-worker-restart 4 --durations=20 --cov=mslib tests` failed. (See above for error)
interesting how pytest.approx does this.
FAILED tests/_test_utils/test_coordinate.py::TestAngles::test_rotate_point[point5-45-rotated_point5] - assert (-1.767766952...7669529663689) == approx((-1.76...78 ± 1.8e-06)) comparison failed. Mismatched elements: 2 / 2: Max absolute difference: 3.3047033631383727e-05 Max relative difference: 1.8694225263081064e-05 Index | Obtained | Expected 0 | -1.7677669529663687 | -1.7678 ± 1.8e-06 1 | 1.7677669529663689 | 1.7678 ± 1.8e-06 ====== 1 failed, 588 passed, 13 skipped, 34 warnings in 470.39s (0:07:50) ====== ERROR conda.cli.main_run:execute(124): `conda run xvfb-run pytest -v -n 6 --dist loadfile --max-worker-restart 4 --durations=20 --cov=mslib tests` failed. (See above for error)
The test needs more digits, "pytest.approx considers the small variations due to floating-point imprecision." Try with 7 digits again
Hi @ReimarBauer,
Thank you for your valuable feedback and suggestions.
Regarding the issue with the pytest.approx comparison, I understand that floating-point precision can sometimes lead to small mismatches in values, as seen in this case.
The updated code for the rotate_point test:
point = [0.0, 2.5]
angle = 45
rotated_point = (-1.7678, 1.7678)
# Use pytest.approx with the desired tolerance
assert coordinate.rotate_point(point, angle) == pytest.approx(rotated_point, rel=1e-6, abs=1e-6)
Please let me know if you have any further suggestions or questions.
Hi @ReimarBauer,
Thank you for pointing this out!
I calculated the expected rotated point (-1.7678, 1.7678) using the standard 2D rotation formula:
x' = x * cos(θ) - y * sin(θ)
y' = x * sin(θ) + y * cos(θ)
For the point [0.0, 2.5] rotated by 45°, the calculation is:
x' = 0 * cos(45°) - 2.5 * sin(45°) = -1.7678
y' = 0 * sin(45°) + 2.5 * cos(45°) = 1.7678
This matches the expected result. However, I agree that using pytest.approx with more precision would better handle floating-point discrepancies. I'll update the test accordingly.
Please let me know if you'd like any further changes!
Thanks again for your feedback.
the codespell news are addressed in https://github.com/Open-MSS/MSS/issues/2599
@saiabhinav001 please have a look on the request by @joernu76
Hi @ReimarBauer,
Thank you for pointing this out. I’ve reviewed @joernu76's feedback and addressed their concern regarding retaining the old tests.
I have retained the original test cases alongside the new ones to ensure comprehensive test coverage. This ensures that both general scenarios and edge cases are thoroughly validated.
tests/_test_utils/test_coordinate.py:152:62: W292 no newline at end of file
That is now new
FAILED tests/_test_utils/test_coordinate.py::test_latlon_points[ref_lats5-ref_lons5-3-expected_lats5-expected_lons5-greatcircle] - assert False
+ where False = all(array([ 0., 5., 10.]) == [0, 5, 10]
Full diff:
+ array([ 0., 5., 10.])
- [
- 0,
- 5,
- 10,
- ])
FAILED tests/_test_utils/test_coordinate.py::test_latlon_points[ref_lats4-ref_lons4-3-expected_lats4-expected_lons4-greatcircle] - assert False
+ where False = all(array([ 0. ... 10. ]) == [0, 5, 10]
Full diff:
+ array([ 0. , 5.00037949, 10. ])
- [
- 0,
- 5,
- 10,
- ])
====== 2 failed, 586 passed, 12 skipped, 34 warnings in 470.70s (0:07:50) ======
@saiabhinav001 @joernu76 sorry for long waiting on that. I´ve no idea what now makes that test showing the difference, maybe a numpy change? Please could you have look on improving the tests results.