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: df.resample('MS', closed='right') incorrectly places bins #55271

Closed
3 tasks done
dalejung opened this issue Sep 25, 2023 · 4 comments · Fixed by #55283
Closed
3 tasks done

BUG: df.resample('MS', closed='right') incorrectly places bins #55271

dalejung opened this issue Sep 25, 2023 · 4 comments · Fixed by #55283
Labels
Milestone

Comments

@dalejung
Copy link
Contributor

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
dti = pd.date_range(start="2020-01-31", freq="1min", periods=6000)
df = pd.DataFrame({'ts': dti}, index=dti)
grouped = df.resample('MS', closed='right')
print(grouped.last())

Issue Description

df.resample('MS', closed='right') now includes a whole days worth of extra minutes.

# print(grouped.last())
2020-01-01	2020-02-01 23:59:00
2020-02-01	2020-02-04 03:59:00

Notice the last time for 2020-01-01 lands on the last minute of the next day.

Previously this would show 2020-02-01 00:00:00.

This appears to the recent M => ME work. is_superperiod("MS", "D") now returns True where it used to be False. Granted that might be correct, but it triggers logic in _adjust_bin_edges that gives us the erroneous results.

Expected Behavior

Should land on midnight. closed semantics just chose which side the bin edges are placed in. It shouldn't shift the binedges 23 hours.

Installed Versions

INSTALLED VERSIONS

commit : 86ecf05
python : 3.11.3.final.0
python-bits : 64
OS : Linux
OS-release : 6.4.12-arch1-1
Version : #1 SMP PREEMPT_DYNAMIC Thu, 24 Aug 2023 00:38:14 +0000
machine : x86_64
processor :
byteorder : little
LC_ALL : None
LANG : en_US.UTF-8
LOCALE : en_US.UTF-8

pandas : 2.2.0dev0+278.g86ecf054a1
numpy : 1.24.3
pytz : 2023.3
dateutil : 2.8.2
setuptools : 65.3.0
pip : 22.2.2
Cython : 0.29.33
pytest : 7.4.0
hypothesis : 6.47.1
sphinx : 6.2.1
blosc : 1.11.1
feather : None
xlsxwriter : 3.1.0
lxml.etree : 4.9.2
html5lib : 1.1
pymysql : 1.0.3
psycopg2 : 2.9.6
jinja2 : 3.1.2
IPython : 8.16.0.dev
pandas_datareader : None
bs4 : 4.12.2
bottleneck : 1.3.7
dataframe-api-compat: None
fastparquet : 2023.4.0
fsspec : 2023.5.0
gcsfs : 2023.5.0
matplotlib : 3.7.1
numba : 0.57.0
numexpr : 2.8.4
odfpy : None
openpyxl : 3.1.2
pandas_gbq : None
pyarrow : 14.0.0.dev183+gaae5438eb
pyreadstat : 1.2.1
python-calamine : None
pyxlsb : 1.0.10
s3fs : 2023.5.0
scipy : 1.10.1
sqlalchemy : 2.0.14
tables : 3.8.0
tabulate : 0.9.0
xarray : 2023.5.0
xlrd : 2.0.1
zstandard : 0.21.0
tzdata : 2023.3
qtpy : None
pyqt5 : None

@dalejung dalejung added Bug Needs Triage Issue that has not been reviewed by a pandas team member labels Sep 25, 2023
@MarcoGorelli
Copy link
Member

Thanks @dalejung for the report, will take a look

Cc @natmokval we'll need to investigate this

@MarcoGorelli
Copy link
Member

MarcoGorelli commented Sep 25, 2023

is_superperiod("MS", "D") now returns True where it used to be False

I'm not seeing this:

In [1]: from pandas.tseries.frequencies import (
   ...:     is_subperiod,
   ...:     is_superperiod,
   ...: )

In [2]: is_superperiod('MS', 'D')
Out[2]: False

EDIT

Ah I see what you mean

In [2]: from pandas.tseries.offsets import MonthBegin

In [3]: from pandas.tseries.frequencies import (
   ...:     is_subperiod,
   ...:     is_superperiod,
   ...: )

In [4]: is_superperiod(MonthBegin(), 'D')
Out[4]: True

Indeed, this looks wrong - MonthBegin doesn't have a corresponding Period, only MonthEnd does

EDIT2

Just noticed that this isn't a regression since 2.1, as the M->ME PR hasn't appeared in any release. In which case, thanks @dalejung for allowing this bug to be fixed before it reaches users!

@MarcoGorelli MarcoGorelli added Regression Functionality that used to work in a prior pandas version and removed Bug Needs Triage Issue that has not been reviewed by a pandas team member labels Sep 25, 2023
@MarcoGorelli MarcoGorelli added this to the 2.1.2 milestone Sep 25, 2023
@MarcoGorelli MarcoGorelli added Bug and removed Regression Functionality that used to work in a prior pandas version labels Sep 25, 2023
@MarcoGorelli MarcoGorelli modified the milestones: 2.1.2, 2.2 Sep 25, 2023
@natmokval
Copy link
Contributor

Cc @natmokval we'll need to investigate this

thanks, I'll take a look at it

@MarcoGorelli
Copy link
Member

Right, this has lead me to uncover #55281 too...

dalejung added a commit to dalejung/pandas that referenced this issue Sep 25, 2023
dalejung added a commit to dalejung/pandas that referenced this issue Jan 23, 2024
dalejung added a commit to dalejung/pandas that referenced this issue Feb 25, 2024
dalejung added a commit to dalejung/pandas that referenced this issue Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants