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.

Sign up for GitHub

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

Jump to bottom

BUG: np.linalg.vector_norm: return correct shape for keepdims #25560

Merged
merged 1 commit into from
Jan 11, 2024
Merged

BUG: np.linalg.vector_norm: return correct shape for keepdims #25560

merged 1 commit into from
Jan 11, 2024

Conversation

jakevdp
Copy link
Contributor

@jakevdp jakevdp commented Jan 9, 2024

Fixes #25559

@jakevdp jakevdp force-pushed the vector-norm branch 2 times, most recently from d550f38 to dde4120 Compare January 9, 2024 20:03
@jakevdp jakevdp mentioned this pull request Jan 9, 2024
Closed
18 tasks
@charris charris changed the title np.linalg.vector_norm: return correct shape for keepdims BUG: np.linalg.vector_norm: return correct shape for keepdims Jan 9, 2024
@charris charris added 00 - Bug 09 - Backport-Candidate PRs tagged should be backported labels Jan 9, 2024
mhvk
mhvk reviewed Jan 9, 2024
View reviewed changes
Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

Thanks for finding and fixing yet another bug! Clearly, added test coverage here was insufficient. One multi-option suggestion in-line, and also a ping to @mtsokol who added this.

numpy/linalg/_linalg.py Show resolved Hide resolved
mhvk
mhvk approved these changes Jan 9, 2024
View reviewed changes
Copy link
Contributor

@mhvk mhvk left a comment

Choose a reason for hiding this comment

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

Given that my single comment was wrong, I can just approve this. Though I'll wait with merging until @mtsokol has had a look.

@jakevdp
Copy link
Contributor Author

jakevdp commented Jan 9, 2024

Thanks!

@mtsokol
Copy link
Member

mtsokol commented Jan 10, 2024 •
edited
Loading

Thank you for the fix! I took the implementation from the existing numpy.array_api module, but I missed this part:

numpy/numpy/array_api/linalg.py

Line 458 in 6e3b923

shape = list(x.shape)

@jakevdp I think the same fix can be applied there.

@jakevdp
Copy link
Contributor Author

jakevdp commented Jan 11, 2024

@jakevdp I think the same fix can be applied there.

I don't think any fix is necessary there, because in that implementation x.shape remains unchanged through the function.

@mhvk
Copy link
Contributor

mhvk commented Jan 11, 2024

Had 2 approvals and the failure is a false positive, so let's get this in. Thanks, @jakevdp!

@mhvk mhvk merged commit 527d535 into numpy:main Jan 11, 2024
60 of 63 checks passed
@mhvk mhvk added this to the 2.0.0 release milestone Jan 11, 2024
@jakevdp jakevdp deleted the vector-norm branch January 11, 2024 01:25
@charris
Copy link
Member

charris commented Jan 14, 2024

Removed the backport-candidate tag. This causes some some downstream churn.

@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhvk mhvk mhvk approved these changes

@mtsokol mtsokol mtsokol approved these changes

Assignees
No one assigned
Labels
00 - Bug component: numpy.linalg
Projects
None yet
Milestone
2.0.0 release
Development

Successfully merging this pull request may close these issues.

BUG: np.linalg.vector_norm with axis=None and keepdims=True returns the wrong shape
4 participants
@jakevdp @mtsokol @mhvk @charris