CLN: remove check_series_type

[jbrockmendel]

Trying to clean up assert_series_equal and avoid usage of _internal_get_values; I'm finding that these functions are weird, and in particular categorical dtype handling is opaque

[jreback]

this is technically an api change, but not worth leaving it around, but can you add a note in the whatsnew

[jorisvandenbossche]

assert_series_equal is a public function, so we should not just remove keyword arguments, I think.

(this broke geopandas CI)

[jorisvandenbossche]

And for subclasses, this keyword can actually be useful. What was the reason you wanted to remove it?

[jbrockmendel]

What was the reason you wanted to remove it?

I'm trying to get rid of the _internal_get_values usages in assert_series_equal and finding the possible parameter combinations make that difficult. Since this one was unused internally, it seemed like a good candidate. If you want to re-introduce it, I have no objection.

[jorisvandenbossche]

How is _internal_get_values and assert_series_equal related? That is only about checking the class type, but it's not related to the dtype?

[jbrockmendel]

How is _internal_get_values and assert_series_equal related?

_internal_get_values is called a couple of times in assert_series_equal (more before this PR). Replacing the _internal_get_values usages with e.g. _values is the goal, but it turns out that breaks sometimes when we have Categorical _values. assert_series_equal has several parameters that change the behavior when we have categorical, and I was trying to narrow those down (see also #32527)

[jorisvandenbossche]

Sorry, that still doesn't explain to me the relation to check_series_type, which basically only is this part of the diff:

-    if check_series_type:
-        # ToDo: There are some tests using rhs is sparse
-        # lhs is dense. Should use assert_class_equal in future
-        assert isinstance(left, type(right))
-        # assert_class_equal(left, right, obj=obj)
+    # TODO: There are some tests using rhs is sparse
+    # lhs is dense. Should use assert_class_equal in future
+    assert isinstance(left, type(right))
+    # assert_class_equal(left, right, obj=obj)

so how is that related to (or required by) the _internal_get_values changes?

[jbrockmendel]

that still doesn't explain to me the relation to check_series_type

It isnt related, check_series_type was an innocent bystander that happens to be unused internally. Again, I have no objection if you want to restore it.

[jorisvandenbossche]

Well, having that the title of the PR didn't give the impression of being an innocent bystander .. ;) (also your "I'm trying to get rid of the _internal_get_values usages in assert_series_equal and finding the possible parameter combinations make that difficult." seemed to indicate this was related, hence my confusion).