DEPR: Series.first() and DataFrame.first() #53419

DeaMariaLeon · 2023-05-27T17:46:42Z

closes BUG: DataFrame.first has unexpected behavior when passing a DateOffset #45908
Tests added and passed if fixing a bug or adding a new feature
All code checks passed.
Added type annotations to new arguments/methods/functions.
Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

I don't know which "whatsnew" I need to edit (which version)

MarcoGorelli

nice one! just got some minor comments

also, we'll need a whatsnew note for 2.1.0

MarcoGorelli · 2023-05-29T09:33:29Z

pandas/core/generic.py

@@ -9162,6 +9162,12 @@ def first(self, offset) -> Self:
        3 days observed in the dataset, and therefore data for 2018-04-13 was
        not returned.
        """
+        # GH45908 & GH#52487


I don't think we need the issue numbers here

MarcoGorelli · 2023-05-29T09:33:56Z

pandas/core/generic.py

@@ -9162,6 +9162,12 @@ def first(self, offset) -> Self:
        3 days observed in the dataset, and therefore data for 2018-04-13 was
        not returned.
        """
+        # GH45908 & GH#52487
+        warnings.warn(
+            "first is deprecated and will be removed in a future version",


shall we advise users what to do instead? something like "please create a mask and filter using .loc instead"

MarcoGorelli · 2023-05-29T09:37:04Z

pandas/tests/frame/methods/test_first_and_last.py


 class TestFirst:
    def test_first_subset(self, frame_or_series):
        ts = tm.makeTimeDataFrame(freq="12h")
        ts = tm.get_obj(ts, frame_or_series)
-        result = ts.first("10d")
-        assert len(result) == 20
+        with tm.assert_produces_warning(FutureWarning, match=deprecated_msg):


could we perhaps just do something like

ts = tm.makeTimeDataFrame(freq="12h") ts = tm.get_obj(ts, frame_or_series) with tm.assert_produces_warning(FutureWarning, match=deprecated_msg): result = ts.first("10d") ts = tm.makeTimeDataFrame(freq="D") ts = tm.get_obj(ts, frame_or_series) with tm.assert_produces_warning(FutureWarning, match=deprecated_msg): result = ts.first("10d")

so that within each assert_produces_warning, there is just a single statement?

MarcoGorelli · 2023-05-29T09:40:03Z

pandas/tests/frame/methods/test_first_and_last.py

+        with tm.assert_produces_warning(FutureWarning, match=deprecated_msg):
+            with pytest.raises(TypeError, match=msg):  # index is not a DatetimeIndex
+                obj.first("1D")


this is fine, though an alternative would be

with ( tm.assert_produces_warning(FutureWarning, match=deprecated_msg), pytest.raises(TypeError, match=msg) ): obj.first('1D')

There's a ruff check for this https://beta.ruff.rs/docs/rules/multiple-with-statements/ , in case you wanted to make a separate issue about it and we can enable it globally in a separate PR

I opened a new issue.

Imo this looks very ugly as soon as one of both statements gets longer. I would prefer keeping them separate for tests at least

@phofl Separate like this you mean?:

with tm.assert_produces_warning(FutureWarning, match=deprecated_msg): with pytest.raises(TypeError, match=msg): # index is not a DatetimeIndex obj.first("1D")

MarcoGorelli · 2023-05-29T09:40:17Z

pandas/tests/frame/methods/test_first_and_last.py

+            expected = frame_or_series(
+                [1] * periods, index=bdate_range(start, periods=periods)
+            )
+            tm.assert_equal(result, expected)


let's take this out of the with context

MarcoGorelli · 2023-05-29T09:41:05Z

pandas/tests/frame/methods/test_first_and_last.py

        # GH#51032
        df = DataFrame(index=pd.DatetimeIndex([]))
-        result = getattr(df, func)(offset=1)
+        result = getattr(df, "last")(offset=1)


if we're doing getattr with a literal, we may as well just write df.last

Yes, thanks

MarcoGorelli · 2023-05-29T13:33:02Z

pandas/tests/frame/methods/test_first_and_last.py

+        ts = tm.makeTimeDataFrame(freq="D")
+        ts = tm.get_obj(ts, frame_or_series)


doesn't look like this was in the original, could we keep it the same please (except for the with context)?

Sorry - copied/pasted more lines than needed. I wasn't adding more tests, but still an error.. :(

MarcoGorelli

looks good to me, thanks @DeaMariaLeon ! leaving open a bit in case anyone has a comment, then will merge

EDIT: sorry there's some failing unit tests - I should've waited for all jobs to finish before approving

doc/source/whatsnew/v2.1.0.rst

pandas/core/generic.py

DeaMariaLeon · 2023-05-30T11:39:49Z

@MarcoGorelli I think the last failing test is not related to these changes.

I separated the test for first() on test_finalize.py thinking that it will make it easy to remove it when it is time.

MarcoGorelli

this is close! we just have a test failure in the docstring validation CI job

================================== FAILURES ===================================
_________________ [doctest] pandas.core.generic.NDFrame.first __________________
9147         >>> ts
9148                     A
9149         2018-04-09  1
9150         2018-04-11  2
9151         2018-04-13  3
9152         2018-04-15  4
9153 
9154         Get the rows for the first 3 days:
9155 
9156         >>> ts.first('3D')
UNEXPECTED EXCEPTION: FutureWarning('first is deprecated and will be removed in a future version. Please create a mask and filter using `.loc` instead')
Traceback (most recent call last):
  File "/home/runner/micromamba/envs/test/lib/python3.10/doctest.py", line 1350, in __run
    exec(compile(example.source, filename, "single",
  File "<doctest pandas.core.generic.NDFrame.first[3]>", line 1, in <module>
  File "/home/runner/micromamba/envs/test/lib/python3.10/site-packages/pandas/core/generic.py", line 9165, in first
    warnings.warn(
FutureWarning: first is deprecated and will be removed in a future version. Please create a mask and filter using `.loc` instead

https://github.com/pandas-dev/pandas/actions/runs/5119510437/jobs/9204861758?pr=53419

Could you check what was done in the past to deal with docstrings of deprecated methods please? Then we can do something similar here

MarcoGorelli · 2023-05-30T16:53:47Z

pandas/tests/generic/test_finalize.py

+        pd.DataFrame({"A": [1, 1, 1, 1]}, pd.date_range("2000", periods=4)),
+    ],
+)
+def test_finalize_first(data):


nice! good call to split this into a separate test

DeaMariaLeon · 2023-05-31T07:21:53Z

Friendly ping again @MarcoGorelli - It looks green

MarcoGorelli

nice one, thanks @DeaMariaLeon !

MarcoGorelli · 2023-06-01T18:18:49Z

merging then, do you want to do a similar one for last?

DeaMariaLeon · 2023-06-02T07:38:50Z

merging then, do you want to do a similar one for last?

Yes.. Thank you @MarcoGorelli.

* Deprecating first() * Added requested changes * removed repeated ts * Update doc/source/whatsnew/v2.1.0.rst * Update pandas/core/generic.py * pre-commit * Separated test for first() on test_finalize.py * Avoiding docstring on CI --------- Co-authored-by: Marco Edward Gorelli <[email protected]> Co-authored-by: MarcoGorelli <[email protected]>

Deprecating first()

8de4ffa

DeaMariaLeon requested a review from MarcoGorelli May 29, 2023 09:19

MarcoGorelli suggested changes May 29, 2023

View reviewed changes

DeaMariaLeon added 2 commits May 29, 2023 15:28

Added requested changes

96ce115

Merge remote-tracking branch 'upstream/main' into deprfirst

ac7707a

MarcoGorelli reviewed May 29, 2023

View reviewed changes

removed repeated ts

75ee3b1

MarcoGorelli approved these changes May 29, 2023

View reviewed changes

doc/source/whatsnew/v2.1.0.rst Outdated Show resolved Hide resolved

pandas/core/generic.py Outdated Show resolved Hide resolved

MarcoGorelli and others added 3 commits May 29, 2023 15:39

Update doc/source/whatsnew/v2.1.0.rst

6cd46c8

Update pandas/core/generic.py

b8e30b4

pre-commit

51e75fe

MarcoGorelli added Deprecate Functionality to remove in pandas Frequency DateOffsets labels May 29, 2023

MarcoGorelli added this to the 2.1 milestone May 29, 2023

Separated test for first() on test_finalize.py

8fe71ff

MarcoGorelli suggested changes May 30, 2023

View reviewed changes

DeaMariaLeon added 2 commits May 30, 2023 20:01

Avoiding docstring on CI

e195758

Merge remote-tracking branch 'upstream/main' into deprfirst

148ddfd

DeaMariaLeon mentioned this pull request Jun 1, 2023

STYLE: Add ruff check to avoid multiple with statements #53436

Closed

MarcoGorelli approved these changes Jun 1, 2023

View reviewed changes

MarcoGorelli merged commit 3e40c3a into pandas-dev:main Jun 1, 2023

DeaMariaLeon deleted the deprfirst branch June 2, 2023 07:38

This was referenced Jun 2, 2023

BUG fixing module first() and DateOffset #52487

Closed

DEPR: List of deprecations to be removed in 3.0 #50578

Open

jrbourbeau mentioned this pull request Jun 13, 2023

Deprecate DataFrame.first and Series.first dask/dask#10352

Merged

2 tasks

MarcoGorelli mentioned this pull request Jun 15, 2023

DEPR: deprecate DataFrame.last and Series.last #53692

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DEPR: Series.first() and DataFrame.first() #53419

DEPR: Series.first() and DataFrame.first() #53419

DeaMariaLeon commented May 27, 2023

MarcoGorelli left a comment

MarcoGorelli May 29, 2023

DeaMariaLeon May 29, 2023

MarcoGorelli May 29, 2023

DeaMariaLeon May 29, 2023

MarcoGorelli May 29, 2023

DeaMariaLeon May 29, 2023

MarcoGorelli May 29, 2023

DeaMariaLeon May 29, 2023

phofl Jun 1, 2023

DeaMariaLeon Jun 1, 2023

MarcoGorelli May 29, 2023

MarcoGorelli May 29, 2023

DeaMariaLeon May 29, 2023

MarcoGorelli May 29, 2023

DeaMariaLeon May 29, 2023

MarcoGorelli left a comment •

edited

Loading

DeaMariaLeon commented May 30, 2023

MarcoGorelli left a comment

MarcoGorelli May 30, 2023

DeaMariaLeon commented May 31, 2023

MarcoGorelli left a comment

MarcoGorelli commented Jun 1, 2023

DeaMariaLeon commented Jun 2, 2023

		ts = tm.makeTimeDataFrame(freq="D")
		ts = tm.get_obj(ts, frame_or_series)

DEPR: Series.first() and DataFrame.first() #53419

DEPR: Series.first() and DataFrame.first() #53419

Conversation

DeaMariaLeon commented May 27, 2023

MarcoGorelli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarcoGorelli left a comment • edited Loading

Choose a reason for hiding this comment

DeaMariaLeon commented May 30, 2023

MarcoGorelli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DeaMariaLeon commented May 31, 2023

MarcoGorelli left a comment

Choose a reason for hiding this comment

MarcoGorelli commented Jun 1, 2023

DeaMariaLeon commented Jun 2, 2023

MarcoGorelli left a comment •

edited

Loading