Skip to content
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

BUG: DatetimeIndex.is_year_start returns incorrect array if .freq == 'MS' #49606

Closed
2 of 3 tasks
rwijtvliet opened this issue Nov 9, 2022 · 5 comments · Fixed by #57494
Closed
2 of 3 tasks

BUG: DatetimeIndex.is_year_start returns incorrect array if .freq == 'MS' #49606

rwijtvliet opened this issue Nov 9, 2022 · 5 comments · Fixed by #57494
Labels
Bug Index Related to the Index class or subclasses Timeseries

Comments

@rwijtvliet
Copy link

rwijtvliet commented Nov 9, 2022

Pandas version checks

  • I have checked that this issue has not already been reported.

  • I have confirmed this bug exists on the latest version of pandas.

  • I have confirmed this bug exists on the main branch of pandas.

Reproducible Example

import pandas as pd

i = pd.date_range('2020-01-01', freq='MS', periods=12)
# DatetimeIndex(['2020-01-01', '2020-02-01', '2020-03-01', '2020-04-01',
#                '2020-05-01', '2020-06-01', '2020-07-01', '2020-08-01',
#                '2020-09-01', '2020-10-01', '2020-11-01', '2020-12-01'],
#               dtype='datetime64[ns]', freq='MS')

# Incorrect result:
i.is_year_start 
# array([False, False, False, False, False, False, False, False, False, False, False, True])

# Correct result when first removing the frequency:
i.freq = None
i.is_year_start
# array([True, False, False, False, False, False, False, False, False, False, False, False])

Issue Description

The .freq property of the DatetimeIndex influences the result of the .is_year_start property. If it is set to MonthStart, the december timestamp is incorrectly recognised as the start of the year.

.is_quarter_start has a similar issue.

The issue does not arise for .freq == 'QS'.

Expected Behavior

Even if i.freq == 'MS', the result of i.is_year_start should start with True, not end with True.

Installed Versions

INSTALLED VERSIONS

commit : 91111fd
python : 3.9.13.final.0
python-bits : 64
OS : Windows
OS-release : 10
Version : 10.0.19045
machine : AMD64
processor : Intel64 Family 6 Model 165 Stepping 2, GenuineIntel
byteorder : little
LC_ALL : None
LANG : None
LOCALE : de_DE.cp1252

pandas : 1.5.1
numpy : 1.23.4
pytz : 2022.6
dateutil : 2.8.2
setuptools : 63.4.1
pip : 22.2.2
Cython : None
pytest : 7.1.3
hypothesis : None
sphinx : 5.3.0
blosc : None
feather : None
xlsxwriter : None
lxml.etree : None
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : 3.1.2
IPython : 8.5.0
pandas_datareader: None
bs4 : 4.11.1
bottleneck : None
brotli : None
fastparquet : None
fsspec : None
gcsfs : None
matplotlib : 3.6.2
numba : None
numexpr : None
odfpy : None
openpyxl : 3.0.10
pandas_gbq : None
pyarrow : None
pyreadstat : None
pyxlsb : None
s3fs : None
scipy : None
snappy : None
sqlalchemy : None
tables : None
tabulate : None
xarray : None
xlrd : None
xlwt : None
zstandard : None
tzdata : None

@rwijtvliet rwijtvliet added Bug Needs Triage Issue that has not been reviewed by a pandas team member labels Nov 9, 2022
@alexanderwertman3
Copy link

I can take this one

@lohmanndouglas
Copy link
Contributor

lohmanndouglas commented Nov 15, 2022

  • I have confirmed this bug exists on the main branch of pandas.

@jbrockmendel

I was investigating this issue, as I understood there is something strange in the function get_start_end_field.

[strange]
If freq == 'MS':
month_kw == 12 (default)
start_month == 12
end_month == 11

[looks ok]
If freq == 'QS':
month_kw == 1
start_month == 1
end_month == 12

[looks ok]
If freq == None:
month_kw == 12 (default - doesn't matter)
start_month == 1
end_month == 12

Needs help to investigate:

We set a month_kw? Why is 12?

I appreciate any help that guides me to figure out more about this issue :)


Update:

I found that in arrays/datetimes.py we are setting the month_kw to 12. I believe that here is the issue.

with QS (works why?)

In the scenario that we have a QS the freq object is QuarterBegin and this object has a startingMonth attribute, thus we set the month_kw to 1. It works!

image

with MS (does not work)

The freq object is MonthBegin that does not have the startingMonth, thus we set month_kw to 12.

image

But why its work with freq = None?

Because the function get_start_end_fieldcheck if freqstr is None. If it is None (line 261) its sets the start month to 1 again.


I will check the tests and certify that this is the right solution.

                if freq:
                    kwds = freq.kwds
-                  month_kw = kwds.get("startingMonth", kwds.get("month", 12))
+                  month_kw = kwds.get("startingMonth", kwds.get("month", 1))

@alexanderwertman3, are you working on it?

@jbrockmendel, @jorisvandenbossche May I open a PR with this solution?

The solution breaks some tests, and need to be improved. -> help wanted

@alexanderwertman3
Copy link

I am going to start investigating the issue this week. It does seem odd that month_kw is set to 12 in get_start_end_field when freq == 'MS'

@alexanderwertman3
Copy link

@lohmanndouglas:
From what I can understand, when freq == 'QS', the QuarterBegin object has a startingMonth attribute, so we correctly set month_kw to 1.
However, when freq == 'MS', the MonthBegin object does not have a startingMonth attribute, so we set month_kw to 12.

I am still looking into as to why this might be the case. Clearly, there is a reason for defaulting month_kw to 12 because the change you made is causing tests to fail.

@jbrockmendel
Copy link
Member

Clearly, there is a reason for defaulting month_kw to 12 because the change you made is causing tests to fail.

which tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Index Related to the Index class or subclasses Timeseries
Projects
None yet
5 participants