-
Notifications
You must be signed in to change notification settings - Fork 92
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
Closes #3886 refactor idx reduction msg #3889
Closes #3886 refactor idx reduction msg #3889
Conversation
19fb1a2
to
4ab8a9c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great! thanks amanda
9392c73
to
50fa6f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the multiple cases of argmax and argmin a bit confusing, and also the runtime creation of functions. But this passes all the tests, and I tried to break it manually, and couldn't. Must be good :)
51a7e06
to
d787291
Compare
This PR adds new
argmin
andargmax
chapel functions to replace theidxReductionMsg
and remove the associated@arkouda.registerND
annotation.Closes #3886 refactor idx reduction msg