-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
CLN: test_nanops.py #49423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLN: test_nanops.py #49423
Conversation
pandas/tests/test_nanops.py
Outdated
("arr_complex_nan_infj", True), | ||
], | ||
) | ||
def test__has_infs_non_float(request, arr, correct, disable_bottleneck): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double underscore
pandas/tests/test_nanops.py
Outdated
], | ||
) | ||
@pytest.mark.parametrize("astype", [None, "f4", "f2"]) | ||
def test__has_infs_floats(request, arr, correct, astype, disable_bottleneck): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same (probably most tests)
pandas/tests/test_nanops.py
Outdated
|
||
@pytest.fixture | ||
def arr_nan_float1(arr_nan, arr_float1): | ||
return np.vstack([arr_nan, arr_float1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't we just copy arr_nan as the second entry, e.g.
@pytest.fixture
def arr_nan_float1(arr_nan):
return np.vstack([arr_nan, np.copy(arr_float)])
if you don't want to use the same object? Makes it a bit clearer. that the arrays have the same content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Since arr_float1
was was redundant to arr_float
, just removed arr_float1
arr_nan = np.take(arr_nan, 0, axis=-1) | ||
arr_nan_nan = np.take(arr_nan_nan, 0, axis=-1) | ||
arr_float_nan = np.take(arr_float_nan, 0, axis=-1) | ||
arr_float1_nan = np.take(arr_float1_nan, 0, axis=-1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove one of both lines? IIUC are_float_nan and arr_float1_nan have the same content
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these need to be separate objects for the test, but I was able to instantiate 2 separate objects with the same fixture
Hopefully addressed all the comments and green |
thx @mroeschke |
Avoid
setup_method
being unnecessarily called on tests that don't require all the DataFrame/array objects that are being created