Skip to content

Commit

Permalink
[BUG] fix faulty BaseObject.__eq__ and deep_equals if an attribut…
Browse files Browse the repository at this point in the history
…e or nested structure contains `float` (#4109)

This fixes `deep_equals` logic and reverse dependency `BaseObject.__eq__` for the case that parameters or nested objects contained an `float`.

Incorrectly, any two floats were considered equal. This is a consequence of erroneous logic to cover the case of `np.nan`.

For more context, also see sktime/skbase#111

Tests were also added to ensure that the `float` and `np.nan` case are both covered correctly.
  • Loading branch information
fkiraly authored Jan 23, 2023
1 parent 2320399 commit 23fea93
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 4 deletions.
11 changes: 7 additions & 4 deletions sktime/utils/_testing/deep_equals.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,8 @@ def ret(is_equal, msg):
return ret(*_tuple_equals(x, y, return_msg=True))
elif isinstance(x, dict):
return ret(*_dict_equals(x, y, return_msg=True))
elif isinstance(x, type(np.nan)):
return ret(
isinstance(y, type(np.nan)), f"type(x)={type(x)} != type(y)={type(y)}"
)
elif _is_np_nan(x):
return ret(_is_np_nan(y), f"type(x)={type(x)} != type(y)={type(y)}")
elif isclass(x):
return ret(x == y, f".class, x={x.__name__} != y={y.__name__}")
elif type(x).__name__ == "ForecastingHorizon":
Expand All @@ -139,6 +137,11 @@ def ret(is_equal, msg):
return ret(True, "")


def _is_np_nan(x):

return isinstance(x, float) and np.isnan(x)


def _tuple_equals(x, y, return_msg=False):
"""Test two tuples or lists for equality.
Expand Down
3 changes: 3 additions & 0 deletions sktime/utils/_testing/tests/test_deep_equals.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
[([([([()])])])],
np.array([2, 3, 4]),
np.array([2, 4, 5]),
3.5,
4.2,
np.nan,
pd.DataFrame({"a": [4, 2]}),
pd.DataFrame({"a": [4, 3]}),
(np.array([1, 2, 4]), [pd.DataFrame({"a": [4, 2]})]),
Expand Down

0 comments on commit 23fea93

Please sign in to comment.