Fix offset __inits__, apply_index dtypes #19142
Conversation
…days add timedelta64[ns] array instead of int64 array
| Travis looks like a moto problem. |
| | ||
| def _apply_index_days(self, i, roll): | ||
| i += (roll % 2) * Timedelta(days=self.day_of_month).value | ||
| nanos = (roll % 2) * Timedelta(days=self.day_of_month).value |
There was a problem hiding this comment.
I would rather
n = Timedelta((roll % 2) * Timedelta(days=self.day_of_month).value) return i + n + Timedelta(days-1) There was a problem hiding this comment.
then pls add a doc-string (same below)
| | ||
| def _apply_index_days(self, i, roll): | ||
| return i + (roll % 2) * Timedelta(days=self.day_of_month - 1).value | ||
| nanos = (roll % 2) * Timedelta(days=self.day_of_month - 1).value |
| _prefix = 'WOM' | ||
| _adjust_dst = True | ||
| | ||
| def __init__(self, n=1, normalize=False, week=None, weekday=None): |
There was a problem hiding this comment.
this doesn't break anything???
There was a problem hiding this comment.
The default kwargs raise ATM.
There was a problem hiding this comment.
where is the test for this? why isn't it raising currently?
…fsets-intarray
…fsets-intarray
Codecov Report
@@ Coverage Diff @@ ## master #19142 +/- ## ========================================= Coverage ? 91.52% ========================================= Files ? 147 Lines ? 48785 Branches ? 0 ========================================= Hits ? 44651 Misses ? 4134 Partials ? 0
Continue to review full report at Codecov.
|
| | ||
| def _apply_index_days(self, i, roll): | ||
| i += (roll % 2) * Timedelta(days=self.day_of_month).value | ||
| nanos = (roll % 2) * Timedelta(days=self.day_of_month).value |
There was a problem hiding this comment.
then pls add a doc-string (same below)
| _prefix = 'WOM' | ||
| _adjust_dst = True | ||
| | ||
| def __init__(self, n=1, normalize=False, week=None, weekday=None): |
There was a problem hiding this comment.
where is the test for this? why isn't it raising currently?
pandas/tseries/offsets.py Outdated
| return self._end_apply_index(i) | ||
| | ||
| def _end_apply_index(self, dtindex): | ||
| """Offsets index to end of Period frequency""" |
Until a few dozen PRs ago all that was passed to |
ok can you add some pls. |
| since |
| ping |
doc/source/whatsnew/v0.23.0.txt Outdated
| | ||
| - Improved error message when attempting to use a Python keyword as an identifier in a ``numexpr`` backed query (:issue:`18221`) | ||
| - :func:`Timestamp.replace` will now handle Daylight Savings transitions gracefully (:issue:`18319`) | ||
| - Bug in :class:`WeekOfMonth` and :class:`LastWeekOfMonth` where default keyword arguments for constructor raised ``ValueError`` (:issue:`19142`) |
There was a problem hiding this comment.
pls move this and the one above to COnversion, don't put things Other unless they obviously don't fit elsewhere. You could make a TimesSeries section if you want (and move appropriate things), but move this for now.
There was a problem hiding this comment.
OK. For future reference, what heuristic should I be using that maps WeekOfMonth.__init__ ValueError to "conversion"?
| thanks! |
2 participants
This PR does two main things:
Change the default kwargs for offset
__init__methods so that the defaults are valid; ATMWeekOfMonth()andLastWeekOfMonth()will both raiseValueError.ATM
Week.apply_indexadds an int64 array to aDatetimeIndex. This happens to have the desired output because the int64 array represents nanoseconds. This explicitly casts the array totimedelta64[ns]to make this explicit. This is a necessary step towards fixing DatetimeIndex + ndarray[int] wrong, reverse op errors #19123.In addition there are two small pieces of refactoring:
Remove
liboffsets.EndMixinsince it is only mixed in toWeek.Use
liboffsets.roll_conventionto de-duplicate some code inliboffsets.shift_months.git diff upstream/master -u -- "*.py" | flake8 --diff