REF/TST: remove overriding tm.assert_foo pattern #54355
Conversation
mroeschke
added
Testing
|
Thanks @jbrockmendel |
| @classmethod | ||
| def assert_frame_equal(cls, left, right, *args, **kwargs): | ||
| return tm.assert_frame_equal(left, right, *args, **kwargs) | ||
| pass |
There was a problem hiding this comment.
This removal is not backward compatible for extensionarrays tests that inherit BaseExtensionTests. IMO this should be deprecated instead?
There was a problem hiding this comment.
Is there a downstream EA that is actually affected or is this just in principal?
If the latter, I'm inclined to be more aggressive with changing the tests than the non-test code. I don't want to get in the business of writing deprecation tests for the tests.
There was a problem hiding this comment.
Every downstream EA that reuses those tests can be affected if they use the same pattern in those classes (eg in a test they override). See the two linked PRs below for two such examples.
Now, I think the question is: is there any downstream EA implementation that actually makes use of those test class methods (i.e. by overriding them, and thus not relying purely on the base tm.assert_.._equal functions). That I don't know (the two examples of affected projects don't do this, and so for them it's an easy fix)
There was a problem hiding this comment.
Looking at some know downstream EAs, there are a few others that will be affected as well in the sense of having to replace self.assert_.. with tm.assert_.., but I didn't see any public one that actually overrides those methods.
Sits on top of #54337