-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TST: indexing/test_floats.py::TestFloatIndexers::test_scalar_non_numeric #25748
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #25748 +/- ##
==========================================
- Coverage 91.25% 91.25% -0.01%
==========================================
Files 172 172
Lines 52977 52977
==========================================
- Hits 48343 48342 -1
- Misses 4634 4635 +1
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #25748 +/- ##
==========================================
- Coverage 91.25% 91.25% -0.01%
==========================================
Files 172 172
Lines 52977 52977
==========================================
- Hits 48343 48342 -1
- Misses 4634 4635 +1
Continue to review full report at Codecov.
|
|
||
# contains | ||
assert 3.0 not in s |
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 guess that maybe this should have been assert 3.0 not in i
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.
what is i?
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.
quick looks seems this is not much more clear than the original. maybe split get/set will be helpful
@@ -61,109 +61,115 @@ def test_scalar_error(self): | |||
s.iloc[3.0] = 0 | |||
|
|||
@ignore_ix | |||
def test_scalar_non_numeric(self): | |||
@pytest.mark.parametrize('index', [ |
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 we already have a fixture for these
|
||
# contains | ||
assert 3.0 not in s |
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.
what is i?
i agree. the main change is to include the cases that were skipped (not in the
i agree that there is basically only two tests here:
also note that it is relatively straightforward to remove iloc completely from this test (and most of the other tests in this module) as it is tested at the start of the module, which make sense for float indexing with iloc. I felt that was out of scope for this PR. A precursor PR to refactor iloc would certainly make the test look less cluttered. |
if you don't mind that would be great. |
superseded by #25866 |
precursor to #25750
This test has some commented out code with a pytest.raises not used as a context manager.
The goal here is primarily to remove the commented out code.
some refactoring but not enough to allow full parametrization.