Skip to content

DEPR: Timestamp.freq #41586

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

Merged
merged 20 commits into from
Jun 7, 2021
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
212 changes: 212 additions & 0 deletions doc/source/reference/offset_frequency.rst

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.3.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,7 @@ Deprecations
- Deprecated special treatment of lists with first element a Categorical in the :class:`DataFrame` constructor; pass as ``pd.DataFrame({col: categorical, ...})`` instead (:issue:`38845`)
- Deprecated behavior of :class:`DataFrame` constructor when a ``dtype`` is passed and the data cannot be cast to that dtype. In a future version, this will raise instead of being silently ignored (:issue:`24435`)
- Deprecated passing arguments as positional (except for ``"method"``) in :meth:`DataFrame.interpolate` and :meth:`Series.interpolate` (:issue:`41485`)
- Deprecated the :attr:`Timestamp.freq` attribute. For the properties that use it (``is_month_start``, ``is_month_end``, ``is_quarter_start``, ``is_quarter_end``, ``is_year_start``, ``is_year_end``), when you have a ``freq``, use e.g. ``freq.is_month_start(ts)`` (:issue:`15146`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't find the explanation of "for properties that use it .., when you have a freq, ..." very clear. Adding an actual example might clarify.

First, I assume people who do ts.is_month_start (with ts being a Timestamp) don't necessarily know that they are relying on some freq.
Also, the attributes are not going away, right? It's only the fastpath for computing it based on the freq that goes away? Are there actually cases that will change behaviour, or is it only performance? (and if only performance, that might not matter much for scalars, or maybe not worth warning about).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there actually cases that will change behaviour, or is it only performance?

Yes, e.g.

dti = pd.date_range("2017-01-01", periods=30, freq="B")
dti[0].is_month_start  # <- True now, False once freq is removed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And in principle we can know when this will change? (depending on the freq, we can know whether it can give different answers?)
Could we warn only in those cases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, will do

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to warn in fewer cases. Still haven't updated the whatsnew note per the first comment here. suggestions on wording?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i agree here. i think we are simply deprecating freq. why do we need to even say anything about is_month_start for example?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bc the result of is_month_start is going to change in some cases

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One consequence of deprecating Timestamp.freq is that when having a timeseries with a freq, the following equivalency won't no longer always hold:

dtidx.is_month_start[0] == dtidx[0].is_month_start

That's a bit unfortunate, I would say, and probably something we should explicitly document.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok for this as a followup documentation (but for 1.3)

- Deprecated passing arguments as positional in :meth:`DataFrame.ffill`, :meth:`Series.ffill`, :meth:`DataFrame.bfill`, and :meth:`Series.bfill` (:issue:`41485`)
- Deprecated passing arguments as positional in :meth:`DataFrame.sort_values` (other than ``"by"``) and :meth:`Series.sort_values` (:issue:`41485`)
- Deprecated passing arguments as positional in :meth:`DataFrame.dropna` and :meth:`Series.dropna` (:issue:`41485`)
Expand Down
20 changes: 20 additions & 0 deletions pandas/_libs/tslibs/offsets.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,26 @@ cdef class BaseOffset:
# if there were a canonical docstring for what is_anchored means.
return self.n == 1

# ------------------------------------------------------------------

def is_month_start(self, _Timestamp ts):
return ts._get_start_end_field("is_month_start", self)

def is_month_end(self, _Timestamp ts):
return ts._get_start_end_field("is_month_end", self)

def is_quarter_start(self, _Timestamp ts):
return ts._get_start_end_field("is_quarter_start", self)

def is_quarter_end(self, _Timestamp ts):
return ts._get_start_end_field("is_quarter_end", self)

def is_year_start(self, _Timestamp ts):
return ts._get_start_end_field("is_year_start", self)

def is_year_end(self, _Timestamp ts):
return ts._get_start_end_field("is_year_end", self)


cdef class SingleConstructorOffset(BaseOffset):
@classmethod
Expand Down
6 changes: 4 additions & 2 deletions pandas/_libs/tslibs/timestamps.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ cdef object create_timestamp_from_ts(int64_t value,
cdef class _Timestamp(ABCTimestamp):
cdef readonly:
int64_t value, nanosecond
object freq
object _freq

cdef bint _get_start_end_field(self, str field)
cdef bint _get_start_end_field(self, str field, freq)
cdef _get_date_name_field(self, str field, object locale)
cdef int64_t _maybe_convert_value_to_local(self)
cdef bint _can_compare(self, datetime other)
cpdef to_datetime64(self)
cpdef datetime to_pydatetime(_Timestamp self, bint warn=*)
cdef bint _compare_outside_nanorange(_Timestamp self, datetime other,
int op) except -1
cpdef void _set_freq(self, freq)
cpdef bint _needs_field_deprecation_warning(_Timestamp self, freq)
3 changes: 3 additions & 0 deletions pandas/_libs/tslibs/timestamps.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ from typing import (
import numpy as np

from pandas._libs.tslibs import (
BaseOffset,
NaT,
NaTType,
Period,
Expand Down Expand Up @@ -57,6 +58,8 @@ class Timestamp(datetime):
fold: int | None= ...,
) -> _S | NaTType: ...

def _set_freq(self, freq: BaseOffset | None) -> None: ...

@property
def year(self) -> int: ...
@property
Expand Down
Loading