-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TST: add test on checking objects to concatenate #40822
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
lgtm |
|
||
|
||
def test_concat_no_items_raises(): | ||
with pytest.raises(ValueError, match="No objects to concatenate"): |
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 there a reference somewhere of discussion on why this should raise rather than return an empty Series/DataFrame?
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.
Came up in #40820
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.
we don't have any other tests for this?
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.
def test_concat_exclude_none(self):
df = DataFrame(np.random.randn(10, 4))
pieces = [df[:5], None, None, df[5:]]
result = concat(pieces)
tm.assert_frame_equal(result, df)
with pytest.raises(ValueError, match="All objects passed were None"):
concat([None, None])
put near this one
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.
we don't have any other tests for this?
By grepping I did not find that concat
is anywhere invoked on an empty list, besides this new test.
$ grep -oPrn 'concat\(\[\]\)' pandas/tests
pandas/tests/reshape/concat/test_concat.py:606:concat([])
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 you put it next to the test as i indicated above
|
||
|
||
def test_concat_no_items_raises(): | ||
with pytest.raises(ValueError, match="No objects to concatenate"): |
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.
we don't have any other tests for this?
@jreback I moved the test just above |
thanks @ivanovmg |
When looking through the enhancement xref #40820,
I noticed that there was no test checking that the ValueError is raised when concatenating an empty sequence.
Added this test.