implement non-controversial cleanup portions of #18762 #18959
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18959 +/- ##
==========================================
+ Coverage 91.57% 91.59% +0.02%
==========================================
Files 150 150
Lines 48964 48920 -44
==========================================
- Hits 44838 44809 -29
+ Misses 4126 4111 -15
Continue to review full report at Codecov.
|
| day_opt : 'start', 'end' | ||
| 'start': returns 1 | ||
| 'end': returns last day of the month | ||
| month : reference month giving the first month of the year |
There was a problem hiding this comment.
I would make this a required parameter and pass it. its more obvious.
There was a problem hiding this comment.
Sounds good. Just pushed.
|
looks fine. do we need additional test coverage of shift_months, roll_yearday to account for not having None's? (I think yes, if nothing broke with this change) |
I'm fine with adding that to the todo list, but don't want this PR to descend into yak shaving. |
| """ | ||
| Given an int64-based datetime index, shift all elements | ||
| specified number of months using DateOffset semantics | ||
|
|
||
| day: {None, 'start', 'end'} | ||
| day: {None, 'start', 'end', 'business_start', 'business_end'} |
There was a problem hiding this comment.
remove the None as a valid argument or is it? also need to fix the code
|
superseded by #18762 |
#18762 touches a lot of code and is currently in limbo pending discussion of
roll_monthdayandroll_convention. This separates out unrelated cleanup portions of that PR, the idea being that making the diff there simpler will make things easier.git diff upstream/master -u -- "*.py" | flake8 --diff