Skip to content

BUG: resample by BusinessHour raises ValueError #13364

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

Closed
wants to merge 1 commit into from

Conversation

scari
Copy link
Contributor

@scari scari commented Jun 4, 2016

I did this during pandas sprint at PyCon 2016. Hope this close #12351
Resampling with BusinessHour took much more consideration I guess.
Please review this. Any comments are welcome.

@codecov-io
Copy link

Current coverage is 84.23%

Merging #13364 into master will increase coverage by <.01%

@@             master     #13364   diff @@
==========================================
  Files           138        138          
  Lines         50724      50727     +3   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          42726      42729     +3   
  Misses         7998       7998          
  Partials          0          0          

Powered by Codecov. Last updated by 103f7d3...2d3be12

@jreback jreback added Bug Resample resample method labels Jun 5, 2016
@@ -2297,6 +2297,17 @@ def test_upsample_daily_business_daily(self):
expected = ts.asfreq('H', how='s').reindex(exp_rng)
assert_series_equal(result, expected)

def test_resample_hourly_business_hourly(self):
ts = pd.Series(index=pd.date_range(start='2016-06-01 03:00:00',
Copy link
Contributor

Choose a reason for hiding this comment

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

add the issue number here as a comment

@jreback
Copy link
Contributor

jreback commented Jul 6, 2016

can you update

elif isinstance(offset, BusinessHour):
# GH12351 - normalize BH freq leads ValueError
first = Timestamp(offset.rollback(first))
last = Timestamp(offset.rollforward(last + offset))
Copy link
Member

Choose a reason for hiding this comment

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

isn't rollback/rollforward returns Timestamp?

@jorisvandenbossche
Copy link
Member

@scari Do you have time to rebase and update this?

@jreback
Copy link
Contributor

jreback commented Sep 9, 2016

can you rebase / update?

@jreback
Copy link
Contributor

jreback commented Nov 16, 2016

can you rebase and move release notes to 0.19.2

@jreback
Copy link
Contributor

jreback commented Dec 21, 2016

can you rebase / move whatsnew to 0.20.0

@jreback
Copy link
Contributor

jreback commented Feb 1, 2017

closing as stale

@jreback jreback closed this Feb 1, 2017
scari added a commit to scari/pandas that referenced this pull request Apr 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Resample resample method
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: resample by BusinessHour raises ValueError
5 participants