Skip to content

Offsets Roundup #18854

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
21 of 39 tasks
jbrockmendel opened this issue Dec 19, 2017 · 2 comments
Closed
21 of 39 tasks

Offsets Roundup #18854

jbrockmendel opened this issue Dec 19, 2017 · 2 comments
Labels
Frequency DateOffsets Master Tracker High level tracker for similar issues Period Period data type Timezones Timezone data dtype

Comments

@jbrockmendel
Copy link
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.

Unanchored Offsets

Quarter start/end offset confusion

Timezone issues

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

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 Tslibs offsets immutable #18224. Assistance requested.

Possible Bugs, needs confirmation:

Misc

  • BUG: Cannot add non-vectorized DateOffset to empty DatetimeIndex #12724 BUG: Cannot add non-vectorized DateOffset to empty DatetimeIndex
  • normalize should be accounted for in __eq__ (BUG: offsets normalize and __eq__ #17689) (closed by fix DateOffset eq to depend on normalize attr #21404)
  • Do Tick classes with normalize=True make sense? (Tick with normalize=True should be disallowed #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.
  • Not all DateOffsets are comparable #8386 missing comparison methods (Implement missing offset comparison methods #18738 should close this)
@mroeschke
Copy link
Member

Still in use @jbrockmendel ?

@jbrockmendel
Copy link
Member Author

I think everything here that is still relevant has its own issue, which should have the Frequency label. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frequency DateOffsets Master Tracker High level tracker for similar issues Period Period data type Timezones Timezone data dtype
Projects
None yet
Development

No branches or pull requests

3 participants