#330 add checks for metrics

[JumpingDino]

Description

Checks are good for metrics calculation. It's a good idea to assure the vectors (y_pred, y_probs) are the same size and don't have NaNs or infs. For this, two functions were created:

• check_arrays_length
• check_array_nan

Fixes #330

Type of change

Create some checks on utils.py and these functions are used in the metrics in metrics.py

How Has This Been Tested?

Creation of tests in test_utils.py

• test_length
• test_inf_values
• test_nan_values

Checklist

• I have read the contributing guidelines
• I have updated the HISTORY.rst and AUTHORS.rst files
• Linting passes successfully : make lint
• Typing passes successfully : make type-check
• Unit tests pass successfully : make tests
• Coverage is 100% : make coverage
• Documentation builds successfully : make doc

[JumpingDino]

When I run the coverage I have the following results:

However, when I look at the data in test_calibration.py, we have some NaNs on our data

1. In what cases I expect to have data with NaNs?
2. Should I refactor these np.nan?
3. What is your recommendation?

[LacombeLouis]

Maybe we need to check that there are no Nans per row? I think that would be a good start! 😄
We should always expect to have Nans in the calibration (Top-Label), since we only re-calibrate the top-label.

[JumpingDino]

Awesome @LacombeLouis !! Thanks for the idea.
I implemented a function that see if the array has only NaNs.
I broke the function check_array_nan in check_array_nan and check_array_inf.
Thanks for the support and feel free to give feedback on code :)

[codecov-commenter]

Codecov Report

All modified lines are covered by tests ✅

Files	Coverage Δ
mapie/metrics.py	100.00% <100.00%> (ø)
mapie/tests/test_metrics.py	100.00% <100.00%> (ø)
mapie/tests/test_utils.py	100.00% <100.00%> (ø)
mapie/utils.py	100.00% <100.00%> (ø)

📢 Thoughts on this report? Let us know!.

[vincentblot28]

Thank you for the PR ! Great job :)

[LacombeLouis]

Thank you for the PR! It's great!

Thinking out loud here: do you think it would be smart to add a check for the metrics where we expect a score between 0 and 1. @JumpingDino @vincentblot28 @thibaultcordier

[thibaultcordier]

Excellent PR, thank you for your contribution! A few suggestions to refine your code:

• Replace all NDArray with ArrayLike.
• Delete some line breaks.

You can use the functionality directly on GitHub to commit/push directly if you wish.

[JumpingDino]

Hi people, actually with the actual code we coverage all the steps, however when I change the check_arrays_length function to use ArrayLike type the check-type throws the errors said above.

1. Could someone explain the major differences of ArrayLike and NDArray as an argument please? Maybe you can give some reference and/or ideas of benefits of using the ArrayLike type.
2. @LacombeLouis I think it makes sense to check the boundaries of the elements of the array for classification tasks!
3. Do you think it makes sense to implement these sanity checks (check_nan, inf, length) in a single function? By this way I expect to have less verbosity over our metrics.py functions, what do you think?

[thibaultcordier]

Hi @JumpingDino,

1. Could someone explain the major differences of ArrayLike and NDArray as an argument please? Maybe you can give some reference and/or ideas of benefits of using the ArrayLike type.

ArrayLike: https://numpy.org/devdocs/reference/typing.html#numpy.typing.ArrayLike

• A Union representing objects that can be coerced into an ndarray.

NDArray: https://numpy.org/devdocs/reference/typing.html#numpy.typing.NDArray

• Can be used during runtime for typing arrays with a given dtype and unspecified shape.

ArrayLike is more generic than NDArray except that ArrayLike must be cast in NDArray to be used. In the end, we use the parameters as an Numpy array. Sometimes, you may have to manipulate pandas type arrays, which is why we want to take into account all types of array.

When I see the error you encountered, I think we can make the assumption that the parameters are already of type NDArray (if this is not the case, we can simply cast it with numpy and use the check_array function test to check).

I've suggested a modification. You can add it directly in GitHub (you have a function that lets you commit suggestions directly in the browser).

3. Do you think it makes sense to implement these sanity checks (check_nan, inf, length) in a single function? By this way I expect to have less verbosity over our metrics.py functions, what do you think?

Indeed, it could make sense to implement them in a single function. But as it stands, your contribution fulfils the desired objective. You are free to make this change if you wish.

[JumpingDino]

• I though about the idea to make the checks from 0 to 1 but this would require me to understand which are classification metrics and I'm kinda worried about some scenarios this may not happen. Maybe we could just finish this issue and we revisit with more advanced checks later.

I think we're good to go right? What do you think?

I'm available for any refinements! and thanks for the knowledge :)

[thibaultcordier]

I though about the idea to make the checks from 0 to 1 but this would require me to understand which are classification metrics and I'm kinda worried about some scenarios this may not happen. Maybe we could just finish this issue and we revisit with more advanced checks later.

I think we're good to go right? What do you think?

I agree with you, we could finish this PR and open a new issue for more advanced checks.

I'm available for any refinements! and thanks for the knowledge :)

Yes, just one more turn: can you add a new line in HISTORY.rst specifying that you are adding new checks, and a new line in CONTRIBUTING.rst adding your name (if you wish)? I'll be approving your PR next :)