Skip to content

DEPR: depreecate uppercase units 'MIN', 'MS', 'US', 'NS' in Timedelta and 'to_timedelta' #57700

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.

Sign up for GitHub

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

Closed
wants to merge 7 commits into from
Closed

DEPR: depreecate uppercase units 'MIN', 'MS', 'US', 'NS' in Timedelta and 'to_timedelta' #57700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
96f0b94
correct def parse_timedelta_unit, add tests, fix test
natmokval Mar 1, 2024
34e2169
add deprecated units to c_DEPR_ABBREVS
natmokval Mar 2, 2024
53cbb78
Merge branch 'main' into depr-uppercase-units-min-ms-ns-us
natmokval Mar 2, 2024
c42376b
fix cython-lint error
natmokval Mar 2, 2024
b16c72f
remove 'MS' from c_DEPR_ABBREVS
natmokval Mar 2, 2024
ee7ef36
Merge branch 'main' into depr-uppercase-units-min-ms-ns-us
natmokval Mar 7, 2024
a53a00d
add a note to v3.0.0, update Timedelta docstring
natmokval Mar 7, 2024
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
1 change: 1 addition & 0 deletions doc/source/whatsnew/v3.0.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ Other Deprecations
- Deprecated :meth:`Timestamp.utcnow`, use ``Timestamp.now("UTC")`` instead (:issue:`56680`)
- Deprecated allowing non-keyword arguments in :meth:`Series.to_markdown` except ``buf``. (:issue:`57280`)
- Deprecated allowing non-keyword arguments in :meth:`Series.to_string` except ``buf``. (:issue:`57280`)
- Deprecated strings ``MIN``, ``MS``, ``US``, and ``NS`` denoting units in :class:`Timedelta` and :func:`to_timedelta` (:issue:`57700`)
-

.. ---------------------------------------------------------------------------
Expand Down
3 changes: 3 additions & 0 deletions pandas/_libs/tslibs/dtypes.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,9 @@ cdef dict c_DEPR_ABBREVS = {
"u": "us",
"N": "ns",
"n": "ns",
"MIN": "min",
"US": "us",
"NS": "ns",
}


Expand Down
13 changes: 13 additions & 0 deletions pandas/_libs/tslibs/timedeltas.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,14 @@ cpdef inline str parse_timedelta_unit(str unit):
return "ns"
elif unit == "M":
return unit
elif unit == "MS":
warnings.warn(
f"\'{unit}\' is deprecated and will be removed in a future version. "
f"Please use \'ms\' instead of \'{unit}\'.",
FutureWarning,
stacklevel=find_stack_level(),
)
unit = "ms"
elif unit in c_DEPR_ABBREVS:
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding another branch is there any reason to not just add these to c_DEPR_ABBREVS?

Copy link
Contributor Author

@natmokval natmokval Mar 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

thank you, I agree, better to add these deprecated units to the dictionary. I made the changes.

Unfortunately, we can't add "MS" to c_DEPR_ABBREVS because we use that dictionary in the definition of to_offset, and in thus casee"MS" means "MonthBegin". For Timedelta, "ms" means milliseconds, so we want to deprecate "MS" for "Milli" to avoid confusion.

I wonder, can we get these changes into 2.2.x? The reason: we deprecated already uppercase strings “MIN”, “MS”, “US”, and “NS” denoting frequencies for Period and offsets in favor of lowercase strings.

Seems like CI failures aren't related to my changes.

Copy link
Member

Choose a reason for hiding this comment

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

we want to deprecate "MS" for "Milli" to avoid confusion.

sorry I might have missed something but where was this discussed? I don't remember it now. "month begin" isn't valid for timedelta, so I think there's no risk of confusion here?

I wonder, can we get these changes into 2.2.x?

there's already 2.2.0 and 2.2.1 out, I think it's too late now

Copy link
Member

Choose a reason for hiding this comment

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

As discussed: by

deprecate "MS" for "Milli"

you meant "deprecate "MS" for milliseconds", not

deprecate "MS" in favour of "Milli"

as I'd interpreted it

All good then 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, my wording wasn't clear enough. I meant: deprecate "MS" for milliseconds".

I updated Timedelta docstring, and added a note to v3.0.0, but I am unsure if we need the note, because using uppercase strings “MIN”, “MS”, “US”, “NS” for units are not documented.

Copy link
Member

Choose a reason for hiding this comment

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

same as in the other PR, shall we mark this as draft until we've enfored the existing deprecations, so that the codebase can get a little cleaner?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, sounds reasonable, I marked this PR as draft.

warnings.warn(
f"\'{unit}\' is deprecated and will be removed in a "
Expand Down Expand Up @@ -1801,6 +1809,11 @@ class Timedelta(_Timedelta):
Values `H`, `T`, `S`, `L`, `U`, and `N` are deprecated in favour
of the values `h`, `min`, `s`, `ms`, `us`, and `ns`.

.. deprecated:: 3.0.0

Units `MIN`, `MS`, `US`, and `NS` are deprecated in favour
of the units `min`, `ms`, `us`, and `ns`.

**kwargs
Available kwargs: {days, seconds, microseconds,
milliseconds, minutes, hours, weeks}.
Expand Down
2 changes: 0 additions & 2 deletions pandas/core/groupby/generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
Union,
cast,
)
import warnings

import numpy as np

Expand All @@ -32,7 +31,6 @@
Substitution,
doc,
)
from pandas.util._exceptions import find_stack_level

from pandas.core.dtypes.common import (
ensure_int64,
Expand Down
20 changes: 20 additions & 0 deletions pandas/tests/indexes/timedeltas/test_timedelta_range.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,23 @@ def test_timedelta_range_deprecated_freq(
expected_values, dtype="timedelta64[ns]", freq=expected_freq
)
tm.assert_index_equal(result, expected)

@pytest.mark.parametrize(
"depr_unit, unit",
[
("MIN", "min"),
("MS", "ms"),
("US", "us"),
("NS", "ns"),
],
)
def test_timedelta_units_MIN_MS_US_NS_deprecated(self, depr_unit, unit):
# GH#52536
depr_msg = (
f"'{depr_unit}' is deprecated and will be removed in a future version."
)

expected = to_timedelta(np.arange(5), unit=unit)
with tm.assert_produces_warning(FutureWarning, match=depr_msg):
result = to_timedelta(np.arange(5), unit=depr_unit)
tm.assert_index_equal(result, expected)
18 changes: 18 additions & 0 deletions pandas/tests/scalar/timedelta/test_constructors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,24 @@ def test_unit_parser(self, unit, np_unit, wrapper):
result = Timedelta(f"2{unit}")
assert result == expected

@pytest.mark.parametrize(
"unit,unit_depr",
[
("min", "MIN"),
("ms", "MS"),
("us", "US"),
("ns", "NS"),
],
)
def test_units_MIN_MS_US_NS_deprecated(self, unit, unit_depr):
# GH#52536
msg = f"'{unit_depr}' is deprecated and will be removed in a future version."

expected = Timedelta(1, unit=unit)
with tm.assert_produces_warning(FutureWarning, match=msg):
result = Timedelta(1, unit=unit_depr)
tm.assert_equal(result, expected)


def test_construct_from_kwargs_overflow():
# GH#55503
Expand Down
1 change: 0 additions & 1 deletion pandas/tests/scalar/timedelta/test_timedelta.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,6 @@ def conv(v):

assert Timedelta("1000") == np.timedelta64(1000, "ns")
assert Timedelta("1000ns") == np.timedelta64(1000, "ns")
assert Timedelta("1000NS") == np.timedelta64(1000, "ns")

assert Timedelta("10us") == np.timedelta64(10000, "ns")
assert Timedelta("100us") == np.timedelta64(100000, "ns")
Expand Down