MAINT: Remove assertRaises from testing

[gfyoung]

Title is self-explanatory. Pretty hefty PR this one.

Partially addresses #15990.

[codecov]

Codecov Report

Merging #16089 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #16089   +/-   ##
=======================================
  Coverage   90.84%   90.84%           
=======================================
  Files         159      159           
  Lines       50771    50771           
=======================================
  Hits        46125    46125           
  Misses       4646     4646

Flag	Coverage Δ
#multiple	88.62% <100%> (ø)	⬆️
#single	40.36% <100%> (ø)	⬆️

Impacted Files	Coverage Δ
pandas/util/testing.py	79.51% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f9c854...51ed9f5. Read the comment docs.

[jreback]

thanks!

awesome job!

[gfyoung]

FYI (for future PR's), tm.assertRaisesRegexp should not be used. Rather, we should use this paradigm:

with pytest.raises(cls) as exc_info:
   ...
exc_info.matches(regexp)

I stumbled upon this while doing this massive PR. Removing assertRaisesRegexp is going to take A LOT more work to replace than it was to replace assertRaises.

[jreback]

hmm, that's going to be a change, though ok with it if its documented :>

can you create a context manager which emulated assertRaisesRegexp (or gives a better name)?

[gfyoung]

hmm, that's going to be a change, though ok with it if its documented :>

Yes, I added that in this PR.

can you create a context manager which emulated assertRaisesRegexp (or gives a better name)?

I think the name is alright, and we do have a context manager for this (it's a port of Python 2.7's implementation).

[jorisvandenbossche]

@gfyoung what is wrong with tm.assertRaisesRegexp ? (why should it not be used)

[gfyoung]

It isn't idiomatic under the pytest framework. Also, it is a port of Python's unittest tm.assertRaisesRegexp (extra bloat to test framework).

[jorisvandenbossche]

Why isn't it idiomatic? I think it is very similar to pytest.raises in usage, but with a more specific functionality (just a different style in the naming)

And it is not because it is a port of a unittest function, that it is bad?

[jreback]

i agree with joris
let's just rename to non camel

[gfyoung]

The docs explicitly address regex matching in the form that I described above. Also, is it not preferable to use the built-in methods instead of maintaining our own version of a port? Does that change anything?

Also, I don't know what you think about this, but what about pytest.warns instead of tm.assert_produces_warning (see here)?

I'm not pushing these very hard (I ask because they are the pytest conventions instead of unittest for example). However, even if we don't make these changes, we could at least document in #15990 why they were not made.

[jorisvandenbossche]

I still think a helper function is more easy to use than what the pytest docs propose to do (but nice docs actually!), but based on what they do, we can probably simplify the current implementation of assertRaisesRegex a lot?

[gfyoung]

The current implementation of assertRaisesRegexp is relatively stripped down I thought. Most of the heavy lifting is done by the context manager. What did you think we could simplify?

[jorisvandenbossche]

What I meant was that wrapping the code snippet you gave above in a new contextmanager, may be less code than the current implementation (thus, using the pytest functionality for asserting the raised error, checking the error message).
But I didn't look in detail whether this would be worth it (actually improve code simplicity), and it may also not cover directly all cases that the current code can handle.

[gfyoung]

Hmmm...I'm not sure how worthwhile that may be since you have to consider both cases when it is used as a context manager (with) statement or a pure function call (context manager used internally). And I don't think wrapping the pytest snippet is flexible enough to avoid duplicating logic across both cases.