Skip to content
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

Merged
merged 6 commits into from
Nov 10, 2022
Merged

CLN: test_nanops.py #49423

merged 6 commits into from
Nov 10, 2022

Conversation

mroeschke
Copy link
Member

Avoid setup_method being unnecessarily called on tests that don't require all the DataFrame/array objects that are being created

@mroeschke mroeschke added Testing pandas testing functions or related to the test suite Clean labels Oct 31, 2022
@mroeschke mroeschke added this to the 2.0 milestone Nov 2, 2022
("arr_complex_nan_infj", True),
],
)
def test__has_infs_non_float(request, arr, correct, disable_bottleneck):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double underscore

],
)
@pytest.mark.parametrize("astype", [None, "f4", "f2"])
def test__has_infs_floats(request, arr, correct, astype, disable_bottleneck):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same (probably most tests)


@pytest.fixture
def arr_nan_float1(arr_nan, arr_float1):
return np.vstack([arr_nan, arr_float1])
Copy link
Member

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.

Copy link
Member Author

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)
Copy link
Member

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

Copy link
Member Author

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

@mroeschke
Copy link
Member Author

Hopefully addressed all the comments and green

@phofl phofl merged commit 51d2bea into pandas-dev:main Nov 10, 2022
@phofl
Copy link
Member

phofl commented Nov 10, 2022

thx @mroeschke

@mroeschke mroeschke deleted the tst/cln/nanops branch November 10, 2022 20:30
codamuse pushed a commit to codamuse/pandas that referenced this pull request Nov 12, 2022
mliu08 pushed a commit to mliu08/pandas that referenced this pull request Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants