Skip to content

ERR: Improve error message in validate_percentile #51475

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

Merged
merged 26 commits into from
Apr 7, 2023

Conversation

ShashwatAgrawal20
Copy link
Contributor

This PR improves the validation of percentiles in pandas. Previously, when an out-of-range value was detected, the code would suggest a wrong value. This suggestion could be confusing to users and was not always necessary. This PR simplifies the suggestion message to directly state that the value should be between 0 and 1.

@ShashwatAgrawal20
Copy link
Contributor Author

@jbrockmendel

@phofl phofl changed the title validate_percentile error message ERR: Improve error message in validate_percentile Feb 20, 2023
@simonjayhawkins simonjayhawkins added the Error Reporting Incorrect or improved errors from pandas label Feb 22, 2023
Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

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

This isn't the test we agreed upon.

Please call the message simply msg and you don't need brackets after match=

@ShashwatAgrawal20
Copy link
Contributor Author

I apologize for the mistake I made earlier.

@ShashwatAgrawal20 ShashwatAgrawal20 requested a review from phofl March 16, 2023 15:35
percentile_array = [-0.5, 0.25, 1.5]
msg = "percentiles should all be in the interval \\[0, 1\\]"
with pytest.raises(ValueError, match=msg):
s.quantile(percentile_array)
Copy link
Member

Choose a reason for hiding this comment

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

There is a file for quantile tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pandas/tests/frame/methods/test_quantile.py one ?

@ShashwatAgrawal20 ShashwatAgrawal20 requested a review from phofl March 25, 2023 14:05
msg = "percentiles should all be in the interval \\[0, 1\\]"
for invalid in [-1, 2, [0.5, -1], [0.5, 2]]:
Copy link
Member

Choose a reason for hiding this comment

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

Why are you deleting those?

Copy link
Contributor Author

@ShashwatAgrawal20 ShashwatAgrawal20 Mar 26, 2023

Choose a reason for hiding this comment

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

@phofl Do I need append the Series test ?

Something like :-

        msg = "percentiles should all be in the interval \\[0, 1\\]"
        for invalid in [-1, 2, [0.5, -1], [0.5, 2]]:
            with pytest.raises(ValueError, match=msg):
                datetime_series.quantile(invalid)

        s = Series(np.random.randn(100))
        percentile_array = [-0.5, 0.25, 1.5]
        with pytest.raises(ValueError, match=msg):
            s.quantile(percentile_array)

@ShashwatAgrawal20 ShashwatAgrawal20 requested a review from phofl March 30, 2023 06:49
@ShashwatAgrawal20
Copy link
Contributor Author

@phofl, Is this ok?

@phofl phofl added this to the 2.1 milestone Apr 7, 2023
@phofl phofl merged commit 8932173 into pandas-dev:main Apr 7, 2023
@phofl
Copy link
Member

phofl commented Apr 7, 2023

thx @ShashwatAgrawal20

@ShashwatAgrawal20 ShashwatAgrawal20 deleted the validate_percentile_51436 branch May 11, 2024 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Error Reporting Incorrect or improved errors from pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: validate_percentile error message wrong with negative percentiles
4 participants