Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

TYP: np.argmin and np.argmax overload changes#28906

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
jorenham merged 1 commit intonumpy:mainfromlvllvl:issue-28641
May 16, 2025

Conversation

lvllvl
Copy link
Contributor

Attempts toclose#28641

@github-actionsGitHub Actions

This comment has been minimized.

Copy link
Member

@jorenhamjorenham left a comment

Choose a reason for hiding this comment

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

Could you explain why you chose to remove some of the overloads, and why you modified the parameter types and return types of the other ones?

@github-actionsGitHub Actions

This comment has been minimized.

@github-actionsGitHub Actions

This comment has been minimized.

@jorenham
Copy link
Member

In case it wasn't clear yet: Fixing#28641 requires narrowing theout typevar bound ofnp.arg{min,max} (andnp.ndarray.arg{min,max}) frombound=NDArray[Any] tobound=NDArray[np.bool | np.integer]. So there's no need to touch the overloads :)

@lvllvllvllvlforce-pushed theissue-28641 branch 4 times, most recently fromd1ffd4f to91ea834CompareMay 14, 2025 21:36
@lvllvllvllvlforce-pushed theissue-28641 branch 9 times, most recently frome536b84 to600499bCompareMay 15, 2025 18:17
@lvllvl
Copy link
ContributorAuthor

In case it wasn't clear yet: Fixing#28641 requires narrowing theout typevar bound ofnp.arg{min,max} (andnp.ndarray.arg{min,max}) frombound=NDArray[Any] tobound=NDArray[np.bool | np.integer]. So there's no need to touch the overloads :)

Thanks for your help, I adjusted tests that started to fail after I changed the stubs.

Do you think additional testing is needed?

jorenham reacted with thumbs up emoji

@lvllvllvllvl marked this pull request as ready for reviewMay 15, 2025 18:41
@lvllvllvllvl requested a review fromjorenhamMay 15, 2025 21:06
@jorenham
Copy link
Member

Do you think additional testing is needed?

It wouldn't hurt to check that this now indeed rejects non-integer (or bool) arrays forout intyping/tests/data/fail

Copy link
Member

@jorenhamjorenham left a comment

Choose a reason for hiding this comment

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

Thenp.arg{min,max} functions look good now 👌🏻.

Since they're a a bit of a package deal with thenp.ndarray.arg{min,max} methods, it would be best to also change those in the same way.

@lvllvllvllvlforce-pushed theissue-28641 branch 2 times, most recently from6842d13 to88cf0c8CompareMay 16, 2025 00:42
@lvllvllvllvlforce-pushed theissue-28641 branch 6 times, most recently from22f4a1b to5932c98CompareMay 16, 2025 17:29
@jorenham
Copy link
Member

I'd prefer it if you would run the test locally: CI isn't free, and each push notifies me :)

@lvllvl
Copy link
ContributorAuthor

I'd prefer it if you would run the test locally: CI isn't free, and each push notifies me :)

oh my mistake, ok will do!

@jorenhamjorenham merged commit7beea21 intonumpy:mainMay 16, 2025
75 of 76 checks passed
@jorenham
Copy link
Member

thanks@lvllvl :)

@lvllvl
Copy link
ContributorAuthor

thanks@lvllvl :)

Thanks for your help, I learned a lot! V fun.

jorenham reacted with rocket emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@jorenhamjorenhamjorenham approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TYP:np.argmin overloads could be more precise
2 participants
@lvllvl@jorenham
[8]ページ先頭
©2009-2025 Movatter.jp