Skip to content

BUG: Fix resample with np.timedelta64 loffset has no effect #18708

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

Licht-T
Copy link
Contributor

@Licht-T Licht-T commented Dec 10, 2017

@pep8speaks
Copy link

pep8speaks commented Dec 10, 2017

Hello @Licht-T! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on December 10, 2017 at 02:51 Hours UTC

@Licht-T Licht-T force-pushed the fix-numpy-timedelta64-loffset-resample branch from 3f2ccbc to 77cba2b Compare December 10, 2017 02:51
@@ -1051,6 +1051,8 @@ def __init__(self, freq='Min', closed=None, label=None, how='mean',

if isinstance(loffset, compat.string_types):
loffset = to_offset(loffset)
elif isinstance(loffset, np.timedelta64):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much better to do the conversion up front

Timedelta already handles these conversions (timedelta and np.timedelta)

offsets should continue to use to_offset
as those are converted to absolute days for Timedelta

loffset=timedelta(minutes=1)).mean()
# GH7687
result1 = s.resample('5min', closed='right', label='right',
loffset=np.timedelta64(1, 'm')).mean()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parameterize

@codecov
Copy link

codecov bot commented Dec 10, 2017

Codecov Report

Merging #18708 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18708      +/-   ##
==========================================
- Coverage    91.6%   91.56%   -0.05%     
==========================================
  Files         153      153              
  Lines       51273    51275       +2     
==========================================
- Hits        46970    46949      -21     
- Misses       4303     4326      +23
Flag Coverage Δ
#multiple 89.42% <100%> (-0.03%) ⬇️
#single 40.68% <0%> (-0.11%) ⬇️
Impacted Files Coverage Δ
pandas/core/resample.py 96.35% <100%> (+0.01%) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/plotting/_converter.py 64.78% <0%> (-1.74%) ⬇️
pandas/util/testing.py 81.82% <0%> (-0.2%) ⬇️
pandas/core/frame.py 97.81% <0%> (-0.1%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 34a8d36...77cba2b. Read the comment docs.

@jreback jreback added Reshaping Concat, Merge/Join, Stack/Unstack, Explode Timedelta Timedelta data type labels Dec 10, 2017
@jreback
Copy link
Contributor

jreback commented Dec 10, 2017

can you also test for some invalid offsets

@jreback
Copy link
Contributor

jreback commented Jan 21, 2018

closing as stale. if you want to continue working, pls ping.

@jreback jreback closed this Jan 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Reshaping Concat, Merge/Join, Stack/Unstack, Explode Timedelta Timedelta data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

loffset has no effect when passing in a numyp.timedelta64
3 participants