Series append raises TypeError #32090
Conversation
pandas/core/series.py
Outdated
| @@ -2530,6 +2530,13 @@ def append(self, to_append, ignore_index=False, verify_integrity=False): | |||
| to_concat.extend(to_append) | |||
| else: | |||
| to_concat = [self, to_append] | |||
| for x in to_concat[1:]: | |||
There was a problem hiding this comment.
can you make this a single check with any(...)
ideally annotate the args above while we're here
There was a problem hiding this comment.
According to @simonjayhawkins here, for(...) fails faster. I guess it makes sense because with if any(isinstance(x, ABCDataFrame) for x in to_concat[1:]): it waits till the list comprehension is built.
There was a problem hiding this comment.
With to_append annotated using Union[Series, Sequence[Series]], the static analysis is throwing this error:
pandas/core/series.py:2539: error: List item 1 has incompatible type "Union[Any, Sequence[Any]]"; expected "Series"
Found 1 error in 1 file (checked 880 source files)
Is there a better type or way to annotate the args?
There was a problem hiding this comment.
waits till the list comprehension is built
i think that would be true for any([isinstance(x, ABCDataFrame) for x in to_concat[1:]]), but shouldnt be for any(isinstance(x, ABCDataFrame) for x in to_concat[1:]).
@simonjayhawkins am i missing something?
There was a problem hiding this comment.
Yeah let's go with any(isinstance(x, ABCDataFrame) for x in to_concat[1:]). I just timed all 3 and this one was the fastest.
What do you think we should print in the msg? to_append.__name__, which will give sequence type if a sequence with DataFrame is passed? or just DataFrame?
| df = pd.DataFrame({"A": [1, 2]}) | ||
| result = df.A.append([df]) | ||
| expected = pd.DataFrame( | ||
| {0: [1.0, 2.0, None, None], "A": [None, None, 1.0, 2.0]}, index=[0, 1, 0, 1] |
There was a problem hiding this comment.
is there anything about this old test that was correct that should be retained?
There was a problem hiding this comment.
I don't think so. That case is testing for the successful appendage of a Series and a DataFrame. Which is exactly what we're trying to raise an error for. So I guess there's nothing right about the case.
What do you think about this @jbrockmendel
Thanks!
|
Hello @hvardhan20! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-03-03 06:14:24 UTC |
simonjayhawkins
added
Error Reporting
doc/source/whatsnew/v1.1.0.rst
Outdated
| @@ -245,7 +245,7 @@ Other | |||
| instead of ``TypeError: Can only append a Series if ignore_index=True or if the Series has a name`` (:issue:`30871`) | |||
| - Set operations on an object-dtype :class:`Index` now always return object-dtype results (:issue:`31401`) | |||
| - Bug in :meth:`AbstractHolidayCalendar.holidays` when no rules were defined (:issue:`31415`) | |||
| - | |||
| - :meth:`Series.append` will now raise a ``TypeError`` when passed a DataFrame or a sequence containing Dataframe (:issue:`31413`) | |||
There was a problem hiding this comment.
can you move this to reshaping
|
thanks @hvardhan20 |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffSeries.append now raises TypeError when a DataFrame or a sequence containing DataFrame is passed.