Skip to content

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

Merged
merged 9 commits into from
Nov 17, 2022
Merged

CLN: Testing and unused #49754

merged 9 commits into from
Nov 17, 2022

Conversation

mroeschke
Copy link
Member

  • Removed references to append
  • Removed unused randbool
  • Reuse fixtures that are defined in pandas/conftest.py

@mroeschke mroeschke added Testing pandas testing functions or related to the test suite Clean labels Nov 17, 2022
Copy link
Member

@MarcoGorelli MarcoGorelli left a 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):
Copy link
Member

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

Copy link
Member Author

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

pandas/pandas/conftest.py

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"

Comment on lines -492 to +453
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):
Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, exactly

@mroeschke mroeschke added this to the 2.0 milestone Nov 17, 2022
@mroeschke mroeschke merged commit 3d8cb79 into pandas-dev:main Nov 17, 2022
@mroeschke mroeschke deleted the cln/misc branch November 17, 2022 23:25
MarcoGorelli pushed a commit to MarcoGorelli/pandas that referenced this pull request Nov 18, 2022
* Remove unused rand_bools

* Remove references to append

* Reuse existing fixtures

* Replace another top level fixture
mliu08 pushed a commit to mliu08/pandas that referenced this pull request Nov 27, 2022
* Remove unused rand_bools

* Remove references to append

* Reuse existing fixtures

* Replace another top level fixture
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.

2 participants