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.

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

compute_signed_angle_2d is overzealous in what it will operate on #437

Open
willGraham01 opened this issue Feb 25, 2025 · 0 comments · May be fixed by #440
Open

compute_signed_angle_2d is overzealous in what it will operate on #437

willGraham01 opened this issue Feb 25, 2025 · 0 comments · May be fixed by #440
Assignees
Labels
bug Something isn't working

Comments

@willGraham01
Copy link
Contributor

willGraham01 commented Feb 25, 2025

Describe the bug
First reported here. compute_signed_angle_2d can theoretically work when v is given as any xarray.DataArray that can be broadcast against u, but it insists on only having space and time dimensions.

Expected behaviour
This function should broadcast across DataArray dimensions, deferring to the xarray implementation.

Additional Context
The validate_reference_vector function itself is very much overkill, in hindsight. If the user provides v as a DataArray, we shouldn't need to perform all the checks that are included in this function, since xarray will throw an error stating the same problem when it tries to broadcast the arrays in the following calculations. These requirements are also listed in the docstring, so if the user encounters such an error, they were already warned (and will have to work quite hard to actively engineer this situation anyway).

I propose to remove this validator function, as it is only used in compute_signed_angle_2d. We can move the tests that still have relevance to the tests for compute_signed_angle_2d itself. As for the case when v is supplied as a np.ndarray, we can handle this with much faster logic inside compute_signed_angle_2d itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 🚧 In Progress
Development

Successfully merging a pull request may close this issue.

1 participant