-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Series.resample fails on NaT index #39229
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
Conversation
pandas/core/resample.py
Outdated
else: | ||
# if index is all NaT, return a single bin | ||
data = [NaT] | ||
bins = np.array([len(ax)]) |
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.
wait why do we have NaT in an index? e.g. why don't we have 0 bins?
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.
Below this is the logic for when there are NaT
along with non-NaT
(lines 1712-1718 here):
if nat_count > 0:
# NaT handling as in pandas._lib.lib.generate_bins_dt64()
# shift bins by the number of NaT
bins += nat_count
bins = np.insert(bins, 0, nat_count)
binner = binner.insert(0, NaT)
labels = labels.insert(0, NaT)
The added logic agrees with this in the case there are all NaT.
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.
i c. can you unify this logic (maybe to a function)?
thanks @rhshadrach |
This will also resolve the core issue of #34998 (comment). cc @jorisvandenbossche
The added test:
currently fails for
freq="M"
, where upsample is used instead of downsample leading to failure since the index is not unique. I can't tell if this is a bug or an operation that should raise. It also fails forDataFrame
, so I think is unrelated to the issue here.cc @jbrockmendel