add match message for pytest.raises() #33144
Conversation
pandas/tests/indexes/common.py
Outdated
| length = len(indices) | ||
| msg = r"delete\(\) missing 1 required positional argument: 'loc'" | ||
| with pytest.raises(TypeError, match=msg): | ||
| # either depending on numpy version | ||
| indices.delete(len(indices)) | ||
| indices.delete() |
There was a problem hiding this comment.
I think you forgot to put length inside the indices.delete(), on L545.
And I think that this also means you need to change the error message.
There was a problem hiding this comment.
i think @MomIsBestFriend is correct here, the unused length is also the source of the linting failure
ShaharNaveh
added
the
Testing
pandas/tests/indexes/common.py
Outdated
|
|
||
| with pytest.raises(TypeError): | ||
| {} in idx._engine | ||
| value in idx |
There was a problem hiding this comment.
we also need to check for value in idx._engine
There was a problem hiding this comment.
Thanks @jbrockmendel added it now.
| with pytest.raises((IndexError, ValueError)): | ||
| # Exception raised depends on NumPy version. | ||
| msg = "index 6 is out of bounds for axis 0 with size 6" | ||
| with pytest.raises(IndexError, match=msg): | ||
| idx.delete(len(idx)) |
There was a problem hiding this comment.
In earlier versions NumPy used to raise ValueError, pandas setup.py has defined min_numpy_version to 1.13.3. Since NumPy 1.13.3, IndexError is being raised
There was a problem hiding this comment.
@sathyz Thank you for looking into this!
pandas/tests/indexes/common.py
Outdated
| with pytest.raises(TypeError, match=msg): | ||
| [] in idx | ||
|
|
||
| msg = ( |
There was a problem hiding this comment.
Can you please use the pattern of:
msg = "|".join([
"foo",
"bar",
"baz"
])
| @@ -105,7 +105,11 @@ def test_unsortedindex(): | |||
| expected = df.iloc[0] | |||
| tm.assert_series_equal(result, expected) | |||
|
|
|||
| with pytest.raises(UnsortedIndexError): | |||
| msg = ( | |||
| r"MultiIndex slicing requires the index to be lexsorted: " | |||
There was a problem hiding this comment.
| r"MultiIndex slicing requires the index to be lexsorted: " | |
| "MultiIndex slicing requires the index to be lexsorted: " |
There was a problem hiding this comment.
A single line is split into multiple lines, to adhere to the formatting rules. If I were to do this, one of the lines will be "" and other will be r"".
| # either depending on numpy version | ||
| indices.delete() | ||
| indices.delete(length) |
There was a problem hiding this comment.
are we testing the missing-argument case somewhere else?
i think the "either depending..." comment is obsolete
There was a problem hiding this comment.
@jbrockmendel mentioned to remove a comment, pls do so and ping on green.
|
I'm not sure if I have broken something in the tests, the failures are from the modules which I haven't touched, waiting for the fixes in master. |
|
LGTM |
|
@jreback should be good to merge now. |
|
thanks @sathyz |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff