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

BUG: throw ValueError for mismatched w dimensions and test for error #18199

Merged
merged 1 commit into from
Mar 31, 2023

Conversation

scb-school
Copy link
Contributor

Reference issue

Closes #17725

What does this implement/fix?

Fixes misleading error message in hamming function when dimensions of optional argument w do not match u and v. Also implements new test in test_distance.py that checks for the error.

Additional information

N/A

@scb-school scb-school changed the title BUG: throw AssertionError for mismatched w dimensions and test for error BUG: throw ValueError for mismatched w dimensions and test for error Mar 27, 2023
@j-bowhay j-bowhay added defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.spatial labels Mar 27, 2023
Copy link
Member

@j-bowhay j-bowhay left a comment

Choose a reason for hiding this comment

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

Thanks, @scb-school! A couple of suggestions. Also, the lint failure looks related, please could you check (python dev.py lint) and fix this.

scipy/spatial/distance.py Outdated Show resolved Hide resolved
scipy/spatial/tests/test_distance.py Outdated Show resolved Hide resolved
@scb-school scb-school requested review from tupui and j-bowhay and removed request for tylerjereddy, peterbell10, tupui and j-bowhay March 28, 2023 19:11
Copy link
Member

@j-bowhay j-bowhay left a comment

Choose a reason for hiding this comment

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

Sorry missed this the first time around but it is important to call _validate_weights(w) before performing the check so we don't break compatibility if the user parses a list

scipy/spatial/distance.py Outdated Show resolved Hide resolved
scipy/spatial/tests/test_distance.py Outdated Show resolved Hide resolved
Copy link
Member

@j-bowhay j-bowhay left a comment

Choose a reason for hiding this comment

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

Thanks @scb-school and congratulations on your first contribution to SciPy, keep them coming!

@j-bowhay j-bowhay added this to the 1.11.0 milestone Mar 31, 2023
@j-bowhay j-bowhay merged commit d431ef6 into scipy:main Mar 31, 2023
@scb-school scb-school deleted the 17798-w-dimensions-error branch May 5, 2023 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.spatial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: spatial: Bad error message from hamming when w has the wrong size.
3 participants