ivy icon indicating copy to clipboard operation
ivy copied to clipboard

Published 20 hours ago • verified publisher icon Transpile-AI

ndarray.max

Open geeythree opened this issue 2 years ago • 5 comments

Close #5641

geeythree avatar Oct 13 '22 13:10 geeythree

Hi @ankitdsi2010 I kindly request you to review the PR.

Additionally, can you please clarify a doubt? image ndarray.max has a parameter initial. I am unsure how <no value> can be implemented here. In the current PR I have not included this parameter. But if I have to implement it, can I use numpys np._NoValue object?

geeythree avatar Oct 14 '22 08:10 geeythree

Yeah you can use the numpy NoValue object and see if it works

ankitdsi2010 avatar Oct 18 '22 13:10 ankitdsi2010

Hi @geeythree :) I'll be reviewing this PR moving forward. Could you ping me when you're done with changes, or simply want a review?

modanesh avatar Oct 29 '22 07:10 modanesh

@modanesh Thanks for letting me know! :) I'm having some trouble figuring out how to write test for the function that I implemented

geeythree avatar Oct 29 '22 07:10 geeythree

Here we describe in detail how tests work and how to write one: https://lets-unify.ai/ivy/deep_dive/ivy_tests.html I highly recommend take a look at it. Don't hesitate to ask any questions :)

modanesh avatar Oct 29 '22 07:10 modanesh

This PR has been labelled as stale because it has been inactive for more than 7 days. If you would like to continue working on this PR, then please add another comment or this PR will be closed in 7 days.

ivy-seed avatar Nov 08 '22 06:11 ivy-seed

I'm still working on this PR.

geeythree avatar Nov 08 '22 12:11 geeythree

@modanesh I see that two methods for max() had been implemented in the frontend file (one which I had written and another which I wasn't aware of), and a test function has already been implemented. Can you please tell me what needs to be done now?

geeythree avatar Nov 12 '22 16:11 geeythree

Could you point out to where those implementations are?

modanesh avatar Nov 14 '22 05:11 modanesh

ivy/ivy/functional/frontends/numpy/ndarray/ndarray.py at line 65 already contains an implementation of max(). I am not sure if this happened when I accidentally force pushed my commits onto the ivy repo or if somebody else had written it.

ivy/ivy_tests/test_ivy/test_frontends/test_numpy/test_ndarray/test_ndarray.py at line 495 already has test implemented for max() which was not written by me.

geeythree avatar Nov 14 '22 06:11 geeythree

I will look into it and will let you know.

modanesh avatar Nov 14 '22 06:11 modanesh

I'm going to close this PR as the function is already implemented. This shouldn't have happened since we have a clear protocol on how to reserve a subtask and work on it, so this is an error on our end. Since @geeythree started working on it, you may proceed to the next stage of the application process :)

modanesh avatar Nov 14 '22 09:11 modanesh

Thank you for the clarification, @modanesh! Just for confirmation, is it ok if I work on another subtask?

geeythree avatar Nov 14 '22 09:11 geeythree

Of course it's fine. We would appreciate your contribution 🙂

modanesh avatar Nov 14 '22 09:11 modanesh