Repository navigation
Make cart2pol and pol2cart refuse 3D data, and test kinematics in 3D - #1147
Open
Mudassiruddin7 wants to merge 1 commit into
Open
Mudassiruddin7 wants to merge 1 commit into
Mudassiruddin7 wants to merge 1 commit into
Conversation
`cart2pol` only checked that `space` contained `x` and `y`. For 3D data it passed the check, and `rho` was then the full 3D norm while `phi` was the 2D angle of the x-y projection, so the polar coordinates did not describe the point and did not round-trip through `pol2cart`. Require that `space` is exactly `x` and `y` (and `space_pol` exactly `rho` and `phi` in `pol2cart`), like `compute_signed_angle_2d` already does. Add tests for the behaviour on x-y-z data that neuroinformatics-unit#363 asked to be checked: velocity, speed, acceleration, path length, straightness, displacements, norm and unit vectors give the right answer, and the 2D-only metrics raise a ValueError.
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
cart2polaccepted 3D data and returned wrong polar coordinates, and the 3D behaviour of the rest of the kinematics functions was untested. This addresses #363.The bug
cart2polcalledvalidate_dims_coords(data, {"space": ["x", "y"]}), which only checks thatxandyare present. With an extrazcoordinate it went on:rho = compute_norm(data)is then the full 3D norm, whilephi = arctan2(y, x)is the angle of the x-y projection. For example, for a helix(cos t, sin t, 0.5 t)it returnsrho = sqrt(1 + 0.25 t^2)instead of 1, andpol2cartdoes not give the input back.It now requires exactly
xandy, the same ascompute_signed_angle_2ddoes withexact_coords=True, andpol2cartrequires exactlyrhoandphi. Both docstrings say so. The error message is the existing one fromvalidate_dims_coords.3D tests
For #363 I ran each function on a straight line (1, 2, 2) and a helix in x-y-z and added
test_kinematics_3d.py:compute_normandconvert_to_unitgive the closed-form result in 3D (they needed no change);compute_turning_angle,compute_directional_change,compute_path_sinuosity,compute_path_emax,compute_forward_vectorandcompute_signed_angle_2draise aValueErrorfor 3D data (also unchanged).test_vector.pygets a 3D case forcart2poland an extra-coordinate case forpol2cart. Those two cases fail onmainand pass now; the other new tests pass on both, and are there to keep the 3D behaviour from changing.What is not done
#363 also suggests a 3D variant of
valid_poses_dataset_uniform_linear_motionand parametrising the existing tests with it. I wrote separate 3D tests instead, because the 2D-only functions would need to be excluded in every parametrised test. Happy to switch if you prefer the fixture.How has this been tested?
The full unit suite passes (1506 passed, 2 xfailed), and the pre-commit hooks (ruff, ruff format, mypy, codespell, check-manifest) pass on the changed files.