Repository: pandas-dev/pandas
Pull request: Remove return values for asserts
Unless they are context managers.

- [X] closes #25135 
- [X] tests added / passed (not applicable)
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Problem statement:
assert_*_equal issues
The function `assert_almost_equal` may return the return value of another functions like `assert_frame_equal` or `assert_series_equal`.
But `assert_frame_equal` does not return any value explicitly, hence it shall always return None.
Also, `assert_series_equal` may or may not return a value explicitly.

Are those assertion functions usable or am I overlooking something?

Hints from issue discussion:
`assert_almost_equal` is not part of the public API, can you please show an example of what's wrong?
I was not sure about how to use `assert_almost_equal`, despite the name starting with `assert` I have looked at the code and have seen return statements, so I figured out I should check the truth of the returned value.
May be the docstring could specify that an `AssertionError` is raised when the assertion fails.

If that's not a public function, then please ignore this issue.

Nevertheless, I think a public function on par with the numpy allclose would be very useful (with the ability to combine absolute and relative errors).
That function is not public - but it does look like the return statements are redundant so will keep this open as a code cleanup issue.  Also would take that improvement to the docstring.
@chris-b1 if I understand this correctly, the assertions part (```assert_frame_equal``` and `assert_series_equal`) will remain as is, but their return values need not be returned? Only code cleanup is the removal of the `return` keyword?

Commit message: Fix broken tests

Patch to review:
diff --git a/pandas/tests/indexes/interval/test_interval.py b/pandas/tests/indexes/interval/test_interval.py
index e4f25ff143273..711313673abfc 100644
--- a/pandas/tests/indexes/interval/test_interval.py
+++ b/pandas/tests/indexes/interval/test_interval.py
@@ -541,12 +541,12 @@ def test_get_loc_datetimelike_overlapping(self, arrays):
         value = index[0].mid + Timedelta('12 hours')
         result = np.sort(index.get_loc(value))
         expected = np.array([0, 1], dtype='intp')
-        assert tm.assert_numpy_array_equal(result, expected)
+        tm.assert_numpy_array_equal(result, expected)
 
         interval = Interval(index[0].left, index[1].right)
         result = np.sort(index.get_loc(interval))
         expected = np.array([0, 1, 2], dtype='intp')
-        assert tm.assert_numpy_array_equal(result, expected)
+        tm.assert_numpy_array_equal(result, expected)
 
     # To be removed, replaced by test_interval_new.py (see #16316, #16386)
     def test_get_indexer(self):
diff --git a/pandas/tests/io/parser/test_textreader.py b/pandas/tests/io/parser/test_textreader.py
index 8119de67890a5..22e0ce775e21f 100644
--- a/pandas/tests/io/parser/test_textreader.py
+++ b/pandas/tests/io/parser/test_textreader.py
@@ -349,5 +349,4 @@ def test_empty_csv_input(self):
 
 def assert_array_dicts_equal(left, right):
     for k, v in compat.iteritems(left):
-        assert tm.assert_numpy_array_equal(np.asarray(v),
-                                           np.asarray(right[k]))
+        tm.assert_numpy_array_equal(np.asarray(v), np.asarray(right[k]))
diff --git a/pandas/tests/series/test_constructors.py b/pandas/tests/series/test_constructors.py
index d92ca48751d0a..fd921b52f4903 100644
--- a/pandas/tests/series/test_constructors.py
+++ b/pandas/tests/series/test_constructors.py
@@ -1009,7 +1009,7 @@ def test_fromDict(self):
         data = {'a': 0, 'b': 1, 'c': 2, 'd': 3}
 
         series = Series(data)
-        assert tm.is_sorted(series.index)
+        tm.is_sorted(series.index)
 
         data = {'a': 0, 'b': '1', 'c': '2', 'd': datetime.now()}
         series = Series(data)
diff --git a/pandas/tests/tools/test_numeric.py b/pandas/tests/tools/test_numeric.py
index 97e1dc2f6aefc..b7c6b8eca2b15 100644
--- a/pandas/tests/tools/test_numeric.py
+++ b/pandas/tests/tools/test_numeric.py
@@ -242,7 +242,7 @@ def test_really_large_scalar(large_val, signed, transform, errors):
     else:
         expected = float(val) if (errors == "coerce" and
                                   val_is_string) else val
-        assert tm.assert_almost_equal(to_numeric(val, **kwargs), expected)
+        tm.assert_almost_equal(to_numeric(val, **kwargs), expected)
 
 
 def test_really_large_in_arr(large_val, signed, transform,
diff --git a/pandas/util/testing.py b/pandas/util/testing.py
index a5ae1f6a4d960..a59ccd9780199 100644
--- a/pandas/util/testing.py
+++ b/pandas/util/testing.py
@@ -286,25 +286,25 @@ def assert_almost_equal(left, right, check_dtype="equiv",
     """
 
     if isinstance(left, pd.Index):
-        return assert_index_equal(left, right,
-                                  check_exact=False,
-                                  exact=check_dtype,
-                                  check_less_precise=check_less_precise,
-                                  **kwargs)
+        assert_index_equal(left, right,
+                           check_exact=False,
+                           exact=check_dtype,
+                           check_less_precise=check_less_precise,
+                           **kwargs)
 
     elif isinstance(left, pd.Series):
-        return assert_series_equal(left, right,
-                                   check_exact=False,
-                                   check_dtype=check_dtype,
-                                   check_less_precise=check_less_precise,
-                                   **kwargs)
+        assert_series_equal(left, right,
+                            check_exact=False,
+                            check_dtype=check_dtype,
+                            check_less_precise=check_less_precise,
+                            **kwargs)
 
     elif isinstance(left, pd.DataFrame):
-        return assert_frame_equal(left, right,
-                                  check_exact=False,
-                                  check_dtype=check_dtype,
-                                  check_less_precise=check_less_precise,
-                                  **kwargs)
+        assert_frame_equal(left, right,
+                           check_exact=False,
+                           check_dtype=check_dtype,
+                           check_less_precise=check_less_precise,
+                           **kwargs)
 
     else:
         # Other sequences.
@@ -322,7 +322,7 @@ def assert_almost_equal(left, right, check_dtype="equiv",
                 else:
                     obj = "Input"
                 assert_class_equal(left, right, obj=obj)
-        return _testing.assert_almost_equal(
+        _testing.assert_almost_equal(
             left, right,
             check_dtype=check_dtype,
             check_less_precise=check_less_precise,
@@ -360,7 +360,7 @@ def _check_isinstance(left, right, cls):
 def assert_dict_equal(left, right, compare_keys=True):
 
     _check_isinstance(left, right, dict)
-    return _testing.assert_dict_equal(left, right, compare_keys=compare_keys)
+    _testing.assert_dict_equal(left, right, compare_keys=compare_keys)
 
 
 def randbool(size=(), p=0.5):
@@ -967,10 +967,11 @@ def isiterable(obj):
 
 
 def is_sorted(seq):
+    """Assert that the sequence is sorted."""
     if isinstance(seq, (Index, Series)):
         seq = seq.values
     # sorting does not change precisions
-    return assert_numpy_array_equal(seq, np.sort(np.array(seq)))
+    assert_numpy_array_equal(seq, np.sort(np.array(seq)))
 
 
 def assert_categorical_equal(left, right, check_dtype=True,
@@ -1168,8 +1169,6 @@ def _raise(left, right, err_msg):
         if isinstance(left, np.ndarray) and isinstance(right, np.ndarray):
             assert_attr_equal('dtype', left, right, obj=obj)
 
-    return True
-
 
 def assert_extension_array_equal(left, right, check_dtype=True,
                                  check_less_precise=False,
@@ -1324,11 +1323,13 @@ def assert_series_equal(left, right, check_dtype=True,
         # .values is an ndarray, but ._values is the ExtensionArray.
         # TODO: Use .array
         assert is_extension_array_dtype(right.dtype)
-        return assert_extension_array_equal(left._values, right._values)
+        assert_extension_array_equal(left._values, right._values)
+        return
 
     elif (is_extension_array_dtype(left) and not is_categorical_dtype(left) and
           is_extension_array_dtype(right) and not is_categorical_dtype(right)):
-        return assert_extension_array_equal(left.array, right.array)
+        assert_extension_array_equal(left.array, right.array)
+        return
 
     else:
         _testing.assert_almost_equal(left.get_values(), right.get_values(),


Code review comments:
diff_hunk: @@ -1324,11 +1323,13 @@ def assert_series_equal(left, right, check_dtype=True,
         # .values is an ndarray, but ._values is the ExtensionArray.
         # TODO: Use .array
         assert is_extension_array_dtype(right.dtype)
-        return assert_extension_array_equal(left._values, right._values)
+        assert_extension_array_equal(left._values, right._values)
+        return
 
     elif (is_extension_array_dtype(left) and not is_categorical_dtype(left) and
           is_extension_array_dtype(right) and not is_categorical_dtype(right)):
-        return assert_extension_array_equal(left.array, right.array)
+        assert_extension_array_equal(left.array, right.array)
+        return
path: pandas/util/testing.py
text: @user1:
same don't need a return

@author:
I tried this earlier but some of the tests failed so I added the returns back. 

@author:
See https://github.com/pandas-dev/pandas/pull/25462/commits/091a00e2aa5a7bb81a16731ad77ca11405e514d6 (this was in response to an earlier PR comment)

diff_hunk: @@ -1009,7 +1009,7 @@ def test_fromDict(self):
         data = {'a': 0, 'b': 1, 'c': 2, 'd': 3}
 
         series = Series(data)
-        assert tm.is_sorted(series.index)
+        tm.is_sorted(series.index)
path: pandas/tests/series/test_constructors.py
text: @user1:
can you rename this routine to tm.assert_is_sorted

diff_hunk: @@ -1324,11 +1323,13 @@ def assert_series_equal(left, right, check_dtype=True,
         # .values is an ndarray, but ._values is the ExtensionArray.
         # TODO: Use .array
         assert is_extension_array_dtype(right.dtype)
-        return assert_extension_array_equal(left._values, right._values)
+        assert_extension_array_equal(left._values, right._values)
+        return
path: pandas/util/testing.py
text: @user1:
Hmm...not sure if this is ideal, but at least it makes it clear `assert_extension_array_equal` doesn't return anything.  Could potentially revisit in a follow-up.

@user2:
Is there actually a reason to return here? Seems like it would unintentionally skip out on subsequent checks

@user1:
@user2 : Not sure, but let's revisit in a follow-up, since the goal is to just refactor.

@user2:
Yea sure makes sense @author if you'd like to tackle

@user3:
you can leave out the return statement. assert returns None which is the same