DEPR: Series/DataFrame.append (#35407) #44539
Conversation
neinkeinkaffee
force-pushed
the
35407-deprecate-append
branch
from
356579d
to
2b818bb
Compare
|
Thanks for the suggestions. I've tried to address them. Could you re-review and let me know your feedback? Cheers! |
|
Question on the failing web and docs: looking at the failing doc build, some of the cases are due to direct calls to append, which I can change into concat. But others are indirect, e.g. |
|
no all usages should be changes in tests and docs except those we r explicitly doing |
|
@jreback Just to make sure I don't misunderstand: given that frame.memory_usage is neither in doc nor test, I shouldn't change the call to series.append to concat there, but instead add a :okwarning: to the ipython codeblock? And likewise not change any other instances were append is called in the pandas code that's neither documentation nor test? |
|
no u do not add okwarning anywhere u need to fix this for example in the code not to output a warning (as this is tested in many tests) |
|
when you push pls merge master |
|
Yes, will do! |
| s = pd.Series([0, 1], index=["a", "b"]).set_flags(allows_duplicate_labels=False) | ||
| msg = "Index has duplicates." | ||
| with pytest.raises(pd.errors.DuplicateLabelError, match=msg): | ||
| func(s) | ||
| pd.concat([pd.Series(0, index=["a", "b"]), s]) |
There was a problem hiding this comment.
can you construct the series outside of the pytest.raises; makes clearer exactly what part of this line is raising
| @@ -538,7 +538,8 @@ def test_partial_set_invalid(self): | |||
| # allow object conversion here | |||
| df = orig.copy() | |||
| df.loc["a", :] = df.iloc[0] | |||
| exp = orig._append(Series(df.iloc[0], name="a")) | |||
| s = Series(df.iloc[0], name="a") | |||
| @@ -179,16 +178,12 @@ def test_dups_index(self): | |||
| tm.assert_frame_equal(result.iloc[10:], df) | |||
|
|
|||
| # append | |||
| result = df.iloc[0:8, :].append(df.iloc[8:]) | |||
| result = concat([df.iloc[0:8, :], df.iloc[8:]]) | |||
There was a problem hiding this comment.
are these redundant with the tests above? this may be better thought of as a targeted test for _append.
pandas/core/frame.py
Outdated
| self.index.memory_usage(deep=deep), index=["Index"] | ||
| ).append(result) | ||
| ) | ||
| result = concat([index_memory_usage, result]) |
There was a problem hiding this comment.
i'd rather keep using _append internally; should be more performant and avoids circular dependency
| @@ -109,7 +110,8 @@ def test_append_sorts(self, sort): | |||
| df1 = DataFrame({"a": [1, 2], "b": [1, 2]}, columns=["b", "a"]) | |||
| df2 = DataFrame({"a": [1, 2], "c": [3, 4]}, index=[2, 3]) | |||
|
|
|||
| with tm.assert_produces_warning(None): | |||
| # GH#35407 | |||
| with tm.assert_produces_warning(FutureWarning): | |||
| result = df1.append(df2, sort=sort) | |||
There was a problem hiding this comment.
if this file is about testing 'append', then these should just be changed to test '_append'
|
@neinkeinkaffee can you merge master |
doc/source/user_guide/merging.rst
Outdated
| While not especially efficient (since a new object must be created), you can | ||
| append a single row to a ``DataFrame`` by passing a ``Series`` or dict to | ||
| ``append``, which returns a new ``DataFrame`` as above. | ||
| You can append a single row in form of a series to a ``DataFrame`` in-place |
There was a problem hiding this comment.
don't show L416, just make the alternative the thing
|
closes #22957? |
|
@jreback I've changed the docs and removed the part about adding rows in-place. However, the Database / Linux_py38_IO pipeline currently fails with |
|
@neinkeinkaffee if you can merge master again; just want to get a clean run here. we do have some occasional test failures on CI (though some recent PRs do address some of these) |
|
thanks @neinkeinkaffee yep very nice! |
DataFrame.appendbehave related to indexes types? #22957