quantile

[dhruvsaxena11]

Hi, I have added the quantile function for Ivy numpy frontend statistical function of todo list issue. close #17737

Thank you

Dhruv Saxena

[Ookamice]

@dhruvsaxena11 Hiya, thank you for your contributions. Please could you refer to the contributor's guide regarding writing testcases for the function you have implemented. Due to internal policies, we only accept contributions that are paired with a unit test.

Let me know if you have any other concerns or question! 😃

[dhruvsaxena11]

@Ookamice

Actually I have made this pull request. So plz kindly review

Thank you

Dhruv Saxena

[Ookamice]

@dhruvsaxena11 Yes, I am aware of your PR involving the addition of a quantile function in the numpy statistical frontend. However, I cannot approve this without the respective testcases being written as well. As I have mentioned previously, it would be a good idea to review the contributor's guide regarding writing testcases for the function you have implemented.

In addition, you should be adding the function into an existing file rather than creating a new file for qunatile. You can check what file it needs to be added in by referencing the numpy documentation as well as the Ivy issue source.

Let me know if you have any other questions or doubts.

[dhruvsaxena11]

@Ookamice

I have updated the file with testcases and as in numpy frontend statistical folder the seperate files are created for each function

so I had also done the same by adding new file for quantile function.

Please kindly review and suggest.

Thank You

[Ookamice]

@dhruvsaxena11 Hiya, whilst some files have only 1 function, it is only because they are not populated yet. If you check out the "averages_and_variances.py", you should see multiple functions within. Please consult "https://numpy.org/doc/stable/reference/routines.statistics.html" on the appropriate files to add your functions to

[dhruvsaxena11]

@Ookamice

Thank You for your help.

I added the quantile function in existing order_statistics.py file as per the numpy docs.

please kindly review and suggest.

Thank You

[Ookamice]

@dhruvsaxena11 Thanks for the prompt changes, please could you consider the following changes before I can give it a proper review:

1. relocate the test functions to its respective file (it should be within the ivy_tests folder directory)
2. remove the redundant quartile.py file from the PR
3. remove redundant imports of the file
4. review how other tests are written and rework your testing logic (unfortuntately, your current logic isn't correct)

At the end of these steps, the PR should only consist of 2 files being changes, one for the frontend function, and one for the respective testcase

PS: I would also recommend writing some local small code samples to ensure that your code is free of syntax errors and basic mistakes and please do consider running the tests locally too. 😸

I can give it a proper deep dive review once this is done 🚀

[dhruvsaxena11]

@Ookamice

Thank you for your help.

I have changed the test directory.

please kindly review and suggest.

Thank You

[Ookamice]

@dhruvsaxena11 Please refer kindly to my previous suggestions. I've made some edits with what I have noticed so far with your PR. If you have any other questions, feel free to ask.

[dhruvsaxena11]

@Ookamice

I have made the changes as per your suggestions. please kindly check and suggest.

Thank You

[Ookamice]

@dhruvsaxena11 Please could you justify the extra imports you have added? Maybe I am missing something here. In addition, please review the testing logic for other frontend functions and see how they differ from yours.

Finally, have you tried running your function and testcase yet?

[Ookamice]

@dhruvsaxena11 Hiya, just dug through the documentations for you a little. (Ivy frontends)

I would advise you to check out the numpy section, in particular the decorators:

@handle_numpy_out
@handle_numpy_dtype
@to_ivy_arrays_and_back
@handle_numpy_casting
@from_zero_dim_arrays_to_scalar

This should help simplify your frontend function logic. In addition, I just had a look through the averages_and_variances.py and the respective tests and it's really a great source of knowledge regarding how Ivy numpy frontends work in general. I hope you'll find them useful. Let me know if you have any specific questions or doubts! 👍

[dhruvsaxena11]

@Ookamice

thank you so much for really guiding me and supporting me.

I tried to change the code and logic as per the given decorators and docs as I changes both test and function code. I also refer the averages_and_variances.py file for my work.

please kindly check and suggest.

Thank You.

[Ookamice]

@dhruvsaxena11 Hiya, thanks for the changes. I'm glad you went through the Ivy Deepdive and found the ivy.quantile function. Could you please explain your use of with with_unsupported_dtypes():?

Also, for testing, I would advice using _statistical_dtype_values to get dtype, values, and axis all from one function. You can see this used in the nanpercentile testcase

[dhruvsaxena11]

@Ookamice

Thank you for your guidance.

with with_unsupported_dtypes(): statement is used to ignore unsupported dtypes in the quantile function.

I have made some changes as per your suggestion for the testcase.

please kindly check and suggest.

Thank You

[Ookamice]

@dhruvsaxena11 I would recommend you to read this topic on supported and unsupported data types which may help your understanding in the correct way to utilise with_unsupported_dtypes().

[dhruvsaxena11]

@Ookamice

I updated the code please kindly check and suggest.

Thank you

[Ookamice]

@dhruvsaxena11 Hiya, thanks for the quick fix, however there are some glaring mistakes that I will list out below:

1. I don't believe it is necessary to use x = ivy.as_array(x) as you have already used @to_ivy_arrays_and_back
2. To avoid confusion, please stick to the argument names declared as according to both the numpy's documentation (more detail here) for your frontend implementation declaration and Ivy's documentation for your usage of ivy.quantile.
3. Ties in with (2.) but make sure you include all the declared flags and arguments in your frontend function declaration in the correct ordering for completeness.
4. Ties in with (2.) but make sure you're supplying all the necessary arguments for ivy.quantile.
5. Currently your test only checks for linear interpolation, ideally your tests should aim to be as robust as possible, please review the numpy's documentation for all the possible interpolation methods (please note that numpy has some methods that aren't found in the current implementation of ivy.quantile. I would heavily encourage opening an issue regarding missing functionality within the Ivy API and for this PR, I believe only focusing on the ones currently supported by ivy.quantile would be most productive.)

If you have any other questions, feel free to ask. 🚀

[dhruvsaxena11]

@Ookamice

Thank you so much for your great guidance.

I have updated the function code and test as per suggestion. please kindly check and suggest.

Thank You

[dhruvsaxena11]

@Ookamice

Thank You for reviews.

I have updated the code as per your suggestions. Please kindly check.

Thank You

[Ookamice]

@dhruvsaxena11 Hiya, glad to see you've also fixed the confusion between interpolation and method.

However, I would suggest you add a short logic that checks for if interpolation is not None and set method to interpolation only after checking if method="linear". This should rule out most cases where a user uses both method and interpolation. Whilst this isn't fully rigorous as a user can specify the method to be linear out of completeness, this is the logic used in numpy's implementation so it should suffice.

Also, there's a minor issue in your testing setup with interpolation=interpolation, I would recommend leaving that to interpolation=None as you shouldn't try to set method and interpolation at the same time

Following this, I believe your frontend function and test function will be sufficiently prepared for genuine debugging by executing test cases and examining test results! 😄

[dhruvsaxena11]

@Ookamice

Thank You so much for your guidance. I have made the changes as suggested. Please kindly check.

Thank You

[Ookamice]

@dhruvsaxena11 Hiya, please review python's documentation on the is operator and amend the logic you have added please. Also, have you been running the pre-commit hook for Ivy? I see some discrepencies in linting that lead me to believe that it has not been ran properly. Let me know if you need any help in this.

[dhruvsaxena11]

@Ookamice

I updated the logic and checked the linting as after some changes it is showing both files are formatted successfully but why it is showing formatting error not able to get.

please kindly check and suggest

Thank You

[Ookamice]

@dhruvsaxena11 Let's not worry about formatting errors for now, I can fix that for you at the end once we have verified all the logic is working.

Have you tried running your testcase yet?

[dhruvsaxena11]

@Ookamice Thank you for your help. As I think the logic for testcase is correct as I also referred the other test functions in the file to create mine.

[Ookamice]

@dhruvsaxena11 It's often hard to spot syntax and basic mistakes when testing by eye as you may assume certain features work the way you expect. Please try running the tests using the command line python -m pytest -v -x "ivy_tests/test_ivy/test_frontends/test_numpy/test_statistics/test_order_statistics.py::test_numpy_quantile" --backend 'all'. This should run the test that you have written. I have just ran your tests and have identified that you're missing the import for @handle_numpy_casting

Let me know if you need any help setting this up locally. (Alternatively, you can follow the instructions here to set up an github codespace https://github.com/unifyai/ivy/pull/16348#issuecomment-1584464972)

[dhruvsaxena11]

@Ookamice

Thank you so much for your help. I runned the test and improved the import error. Please kindly check.

Thank You

[Ookamice]

@dhruvsaxena11 Please let me know if all the tests pass successfully. If not, I can guide you if you leave screenshots of the error logs. Whilst I can run the tests and inform you of the error each time, this will add a very long latency in the bug troubleshooting process.

[dhruvsaxena11]

@Ookamice this error is coming

ImportError while loading conftest '/workspaces/ivy/ivy_tests/test_ivy/conftest.py'. ivy_tests/test_ivy/conftest.py:7: in import ivy_tests.test_ivy.helpers.test_parameter_flags as pf ivy_tests/test_ivy/helpers/init.py:10: in from . import testing_helpers ivy_tests/test_ivy/helpers/testing_helpers.py:12: in import ivy.functional.frontends.numpy as np_frontend ivy/functional/frontends/numpy/init.py:487: in from . import statistics ivy/functional/frontends/numpy/statistics/init.py:3: in from . import order_statistics ivy/functional/frontends/numpy/statistics/order_statistics.py:118: in def quantile( ivy/functional/frontends/numpy/func_wrapper.py:159: in handle_numpy_dtype dtype_pos = list(inspect.signature(fn).parameters).index("dtype") E ValueError: 'dtype' is not in list