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

Offsets Roundup #18854

Open
jbrockmendel opened this Issue Dec 19, 2017 · 0 comments

Comments

Projects
None yet
2 participants
@jbrockmendel
Member

jbrockmendel commented Dec 19, 2017

Confusion about offsets with Period. AFAICT the underlying issue is that DateOffset are for the most part intended to be used with date_range and its just a coincidence that some of them work with Period.

  • #4591 Unclear documentation for Period Construction
  • #5091 Missing Periods for some DateOffsets
  • #12379 No str for FY5253Quarter
  • #4878 Issues Creating Period with DateOffset as freq, e.g. pd.Period('2013-12', freq='pd.offsets.BusinessMonthEnd()) raises

Unanchored Offsets

Quarter start/end offset confusion

  • #17787 Anchored offset Starting Quarter and Year defaults to JAN not DEC
  • #2885 Default for "Q" is not the same as default for "QuarterEnd"
  • #5307 Parameter Name of startingMonth for QuarterEnd is Misleading
  • #18235 Quarter.onOffset looks fishy
  • #8435 DEPR: QuarterBegin and BQuarterBegin return days that are not quarter beginnings

Timezone issues

  • #12156 Adding DateOffset(days=1) produces NonExistentTimeError, given example is pd.date_range(pd.Timestamp('2015-1-1', tz='US/Eastern'), pd.Timestamp('2016-1-1', tz='US/Eastern'), freq='H') + pd.DateOffset(days=1)
  • #16980 API: Wrong DateOffset behaviour with DST changes. I think the OP wants Day to behave like relativedelta(days=1) rather than timedelta(hours=24)
  • Lots of errors surfaced when running tests with hypothesis in #18761

RelativeDelta issues. Some of these may be solved or affected by #18329, #18226

  • #7418 BUG: hour/hours (and other plurals) should be the same as singulars. DateOffset behavior here just passes through to dateutil.relativedelta, so the issue seems to be confusingly similar kwarg options.
  • #7707 MonthOffset() gives increase of 1 day instead of 1 month (This raises NotImplementedError)
  • #4804 goes here too

Perf

  • DateOffset.__eq__ (called from Period.__eq__) is very slow because DateOffset._params is very slow. This could be improved a lot if DateOffset were immutable. Attempts to do this so far have run into pickle problems, see #18224. Assistance requested.

Possible Bugs, needs confirmation:

  • #18510 Week(weekday=None).onOffset will always return False, while I think it should always be True. (Closed by #18875)
  • A bunch of offsets fail to satisfy offset.onOffset(ts) \Leftrigharrow (ts + offset) - offset == ts. I think the latter is effectively the definition of onOffset in the general case, need confirmation that this is a bug.

Misc

  • #12724 BUG: Cannot add non-vectorized DateOffset to empty DatetimeIndex
  • normalize should be accounted for in __eq__ (#17689) (closed by #21404)
  • Do Tick classes with normalize=True make sense? (#21434)
  • CacheableOffset mixin is never used
  • liboffsets.WeekDay is not used outside of tests
  • DateOffset._should_cache is never true
  • DateOffset.isAnchored needs a docstring
  • I screwed up a while back confusing BusinessMixin.offset with DateOffset._offset. No real harm done, but should be reverted. Might be worth finding a different name than offset
  • BusinessHourMixin.apply raises an ApplyTypeError with an incomplete error message
  • remove camelCase
  • WeekOfMonth._from_name has a comment # TODO: handle n here..., same with LastWeekOfMonth
  • BQuarterBegin has a comment # I suspect this is wrong for *all* of them, could really use some clarification.
  • YearOffset._get_offset_day has a comment suggesting a more performant implementation may be available.
  • FY5253.apply has a couple of assert False statements for cases which presumably we don't expect to get hit. These should probably raise with a useful error message (or better yet, confirm that they will never get reached.
  • Does Tick need its own apply_index?
  • Easter.apply handles n==0 differently from all others.
  • #8386 missing comparison methods (#18738 should close this)

gfyoung added a commit that referenced this issue Jun 13, 2018

david-liu-brattle-1 added a commit to david-liu-brattle-1/pandas that referenced this issue Jun 18, 2018

david-liu-brattle-1 added a commit to david-liu-brattle-1/pandas that referenced this issue Jun 19, 2018

alimcmaster1 added a commit to alimcmaster1/pandas that referenced this issue Jun 28, 2018

alimcmaster1 added a commit to alimcmaster1/pandas that referenced this issue Jun 28, 2018

Sup3rGeo added a commit to Sup3rGeo/pandas that referenced this issue Oct 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment