Add match=msg to pytest.raises or convert to external_error_raised for modules in tests/plotting #38684
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ing/test_boxplot_method.py
…ing/test_frame_color.py
…ing/test_series.py and fixed one error message from "None doesn't allow negative values" to "pie plot doesn't allow negative values"
…ing/test_hist_method.py or convert them to tm.external_error_raised
…ing/frame/test_frame.py or convert them to tm.external_error_raised. Clarify error message in LinePlot._get_stacked_values
moink
commented
Dec 24, 2020
| "all positive or negative." | ||
| f"{label} contains both positive and negative values" | ||
| "all positive or all negative. " | ||
| f"Column '{label}' contains both positive and negative values" |
There was a problem hiding this comment.
I think this makes the error message a bit clearer.
moink
commented
Dec 24, 2020
| @@ -1555,7 +1555,7 @@ class PiePlot(MPLPlot): | |||
| def __init__(self, data, kind=None, **kwargs): | |||
| data = data.fillna(value=0) | |||
| if (data < 0).any().any(): | |||
| raise ValueError(f"{kind} doesn't allow negative values") | |||
| raise ValueError(f"{self._kind} plot doesn't allow negative values") | |||
There was a problem hiding this comment.
In the test, kind was None, and I don't think that was the intent.
moink
commented
Dec 24, 2020
| df.plot(yerr=np.random.randn(11)) | ||
|
|
||
| df_err = DataFrame({"x": ["zzz"] * 12, "y": ["zzz"] * 12}) | ||
| with pytest.raises((ValueError, TypeError)): | ||
| with tm.external_error_raised(TypeError): |
There was a problem hiding this comment.
not sure why this previously allowed it to be a ValueError as well as a TypeError
|
thanks @moink FYI on an issue like this use xref rather than closes in the PR header, otherwise GH will acually close the issue (which you normally want, but this issue is not closed yet). |
Ok thanks |
luckyvs1
pushed a commit
to luckyvs1/pandas
that referenced
this pull request
Jan 20, 2021
…r modules in tests/plotting (pandas-dev#38684)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request xref #30999 to remove bare
pytest.raises. It doesn't close that issue as I have only addressed files withinpandas/tests/plotting/.Things I did that were not simple adding
match = msgtopytest.raisesor converting them totm.external_error_raised:pandas/plotting/_matplotlib/_core.pyI found two error messages somewhat unclear. I changed the error message in theValueErrorraised by_get_stacked_valuesto be, imo, a little bit clearer. In addition, in the constructor forPiePlot, the previous error message for theValueErrorwas "None doesn't allow negative values" which I changed to "pie plot doesn't allow negative values" because I believe that was the original intent.test_frame.py, in thetest_errorbar_plotmethod, thepytest.raiseswas allowing matplotlib to raise either aTypeErroror aValueError. I changed it to anexternal_error_raisedbut also specified it to be aTypeErrorbecause that is what it is, and I didn't see the reason it was allowed to be either error.test_frame.py, the testtest_invalid_xy_argswas pytest-parameterized to test two different kinds of errors: x not being a label or position, or the label being bad. I split it into two tests, calling the new testtest_bad_labeltest_frame.py, the testtest_partially_invalid_plot_datalooped over a list with one element in it. I guessed that this was a remnant of an older test strategy that is no longer employed. I removed the list and made the kind of plot being tested more explicit.pytest.raisesbecause they were setup lines that didn't raise the error: I wanted to make it clear exactly which pandas method was raising the error.I will note that another developer, DylanBrdt commented on the issue that they would address these files but they haven't been active on github since February.
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff