-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DEPR: Deprecate passing range-like arguments to DatetimeIndex, TimedeltaIndex #23919
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
Changes from 12 commits
f8bed85
5d8a107
28e2184
0f75b9d
2c40c3a
d67f87c
afdab5b
8f435ed
4d7c9e2
2e587e3
0469b74
43a52fc
f4e281e
15e6c30
5dc66f3
b931878
07bfc45
e209a81
d6df7a3
eb5d9c5
cc40717
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
""" implement the TimedeltaIndex """ | ||
from datetime import datetime | ||
import warnings | ||
|
||
import numpy as np | ||
|
||
|
@@ -131,10 +132,18 @@ def __new__(cls, data=None, unit=None, freq=None, start=None, end=None, | |
periods=None, closed=None, dtype=None, copy=False, | ||
name=None, verify_integrity=True): | ||
|
||
if verify_integrity is not True: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why isn't the default of verify_integrity None? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because by default we do verify the integrity of a passed frequency. Best guess as to initial motivation is that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right but aren't you deprecating it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. In the future it will just be set to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We still need the variable to exist because there are cases in which we can determine it is not necessary, in which case we can skip a potentially-expensive check. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't recall that (and dont see how it would work), but yah, not passing freq would make verify_integrity unnecessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@jbrockmendel Isn't the end goal to actually remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eventually verify-integrity will not be in the signature. The first line of the method will just define it to be True. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, but then as @jreback said, we should put it at None so we can also deprecate the case for somebody setting it to True explicitly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
warnings.warn("The 'verify_integrity' argument is deprecated, " | ||
"will be removed in a future version.", | ||
FutureWarning, stacklevel=2) | ||
|
||
freq, freq_infer = dtl.maybe_infer_freq(freq) | ||
|
||
if data is None: | ||
# TODO: Remove this block and associated kwargs; GH#20535 | ||
warnings.warn("Creating a TimedeltaIndex by passing range " | ||
"endpoints is deprecated. Use " | ||
"`pandas.timedelta_range` instead.", | ||
FutureWarning, stacklevel=2) | ||
result = cls._generate_range(start, end, periods, freq, | ||
closed=closed) | ||
result.name = name | ||
|
@@ -727,5 +736,8 @@ def timedelta_range(start=None, end=None, periods=None, freq=None, | |
if freq is None and com._any_none(periods, start, end): | ||
freq = 'D' | ||
|
||
return TimedeltaIndex(start=start, end=end, periods=periods, | ||
freq=freq, name=name, closed=closed) | ||
freq, freq_infer = dtl.maybe_infer_freq(freq) | ||
result = TimedeltaIndex._generate_range(start, end, periods, freq, | ||
closed=closed) | ||
result.name = name | ||
return result |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ | |
|
||
import pandas as pd | ||
from pandas import (DataFrame, date_range, Index, | ||
Series, MultiIndex, Timestamp, DatetimeIndex) | ||
Series, MultiIndex, Timestamp) | ||
from pandas.core.groupby.ops import BinGrouper | ||
from pandas.compat import StringIO | ||
from pandas.util import testing as tm | ||
|
@@ -374,9 +374,9 @@ def sumfunc_value(x): | |
expected.reset_index(drop=True)) | ||
|
||
def test_groupby_groups_datetimeindex(self): | ||
# #1430 | ||
# GH#1430 | ||
periods = 1000 | ||
ind = DatetimeIndex(start='2012/1/1', freq='5min', periods=periods) | ||
ind = pd.date_range(start='2012/1/1', freq='5min', periods=periods) | ||
df = DataFrame({'high': np.arange(periods), | ||
'low': np.arange(periods)}, index=ind) | ||
grouped = df.groupby(lambda x: datetime(x.year, x.month, x.day)) | ||
|
@@ -385,7 +385,7 @@ def test_groupby_groups_datetimeindex(self): | |
groups = grouped.groups | ||
assert isinstance(list(groups.keys())[0], datetime) | ||
|
||
# GH 11442 | ||
# GH#11442 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not important, but was just wondering: why are you adding the '#' everywhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mostly for internal consistency (not a big deal, but for grepping purposes). A little bit because I was curious how long it would take before someone asked about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we have have the gh-xxxx style as well, slight prefernce for that |
||
index = pd.date_range('2015/01/01', periods=5, name='date') | ||
df = pd.DataFrame({'A': [5, 6, 7, 8, 9], | ||
'B': [1, 2, 3, 4, 5]}, index=index) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can check
__reduce__
,d
should be_data
and the attributes dictThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea, that seems to work; just pushed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this broke the legacy pickle tests; reverted.