-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
CLN: Testing and unused #49754
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: Testing and unused #49754
Conversation
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.
Looks good, just left a couple of comments to check my understanding
@pytest.mark.parametrize("keep", ["first", "last", False]) | ||
def test_drop_duplicates_series_vs_dataframe(keep): |
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.
cause this is the same as the fixture? 🤔 might be an idea for an automated test
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.
Yup same as this one
Lines 304 to 310 in cca262d
@pytest.fixture(params=["first", "last", False]) | |
def keep(request): | |
""" | |
Valid values for the 'keep' parameter used in | |
.duplicated or .drop_duplicates | |
""" | |
return request.param |
Yeah it would be great to have a check to answer "Does a @pytest.mark.parametrize return the same params as an existing fixture"
def test_maybe_promote_string_with_any(string_dtype, any_numpy_dtype_reduced): | ||
def test_maybe_promote_string_with_any(string_dtype, any_numpy_dtype): |
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.
is the idea here just that any_numpy_dtype
is a superset of any_numpy_dtype_reduced
, and that one might as well test over the larger set?
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.
Yup, exactly
* Remove unused rand_bools * Remove references to append * Reuse existing fixtures * Replace another top level fixture
* Remove unused rand_bools * Remove references to append * Reuse existing fixtures * Replace another top level fixture
append
randbool
pandas/conftest.py