-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: freq setter for DatetimeIndex/TimedeltaIndex/PeriodIndex is poorly supported #20678
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
Comments
Thanks, I thought we already had a similar issue, but can't find it. Additionally, this even fails:
because
+1 Regarding adding a |
I don't think there is an actual case for setting this (though don't completely know). Should deprecate setting and raise and exception. (and of course Indexes are immutable), though we allow setting of 'name' (e.g. meta-data), but this is not just descriptive, but can affect ops. Rather using |
Would creating a new |
I don't really know as well. But in principle, you can create a regular DTI from values, and then it has no freq but it has an inferred freq. And in that case you can set the freq manually (just as you can do with the constructor), to avoid that it is always inferred?
Given the limited use case, I don't think it is worth adding a new method for that (+ that I generally don't like |
What's the status on this? 0.23 or pushing? |
@TomAugspurger : FWIW I plan to push changes that address the review comments later tonight. Not sure how much discussion/additional changes will be required beyond that though. I'll have time this weekend to do work related to it. |
1. Setting to a frequency string alias fails for all datetimelike indexes (same error for all three):
2.
DatetimeIndex
andTimedeltaIndex
allow setting to invalid frequencies:Note that trying to construct this from scratch fails, as expected:
3.
PeriodIndex
gives nonsensical output when setting with an offset:In addition to the nonsensical dates, note the incompatibility in
[13]
betweendtype
andfreq
.Expected Output
I don't know that setting
freq
for aPeriodIndex
should be supported: there's some ambiguity in regards aligning to the start/end of a period, and there's also thePeriodIndex.asfreq
method that gives more control over this.If setting
freq
is disallowed forPeriodIndex
, should setting also be disallowed forDatetimeIndex
andTimedeltaIndex
in order to remain consistent? If so, should there then be an analogousasfreq
for these two?My thoughts on the three issues:
String aliases should be coerced as appropriate (be it via setting or a
asfreq
method).This should raise with a similar message as the constructor (be it via setting or a
asfreq
method).This should raise, potentially with a message indicating to use
asfreq
. Or if we choose to allow setting, this should be consistent with the defaultasfreq
options.Output of
pd.show_versions()
INSTALLED VERSIONS
commit: fa231e8
python: 3.6.1.final.0
python-bits: 64
OS: Windows
OS-release: 10
machine: AMD64
processor: Intel64 Family 6 Model 78 Stepping 3, GenuineIntel
byteorder: little
LC_ALL: None
LANG: None
LOCALE: None.None
pandas: 0.23.0.dev0+740.gfa231e8
pytest: 3.1.2
pip: 9.0.1
setuptools: 39.0.1
Cython: 0.25.2
numpy: 1.13.3
scipy: 1.0.0
pyarrow: 0.6.0
xarray: 0.9.6
IPython: 6.1.0
sphinx: 1.5.6
patsy: 0.4.1
dateutil: 2.6.0
pytz: 2017.2
blosc: None
bottleneck: None
tables: 3.4.2
numexpr: 2.6.4
feather: 0.4.0
matplotlib: 2.0.2
openpyxl: 2.4.8
xlrd: 1.1.0
xlwt: 1.3.0
xlsxwriter: 0.9.8
lxml: 3.8.0
bs4: None
html5lib: 0.999
sqlalchemy: 1.1.13
pymysql: None
psycopg2: None
jinja2: 2.9.6
s3fs: None
fastparquet: 0.1.0
pandas_gbq: None
pandas_datareader: None
The text was updated successfully, but these errors were encountered: