Skip to content

CLN: Refactor pd.offsets.CustomBusinessDay #8458

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
bjonen opened this issue Oct 4, 2014 · 3 comments
Closed

CLN: Refactor pd.offsets.CustomBusinessDay #8458

bjonen opened this issue Oct 4, 2014 · 3 comments
Labels
Datetime Datetime data dtype Frequency DateOffsets good first issue Internals Related to non-user accessible pandas implementation

Comments

@bjonen
Copy link
Contributor

bjonen commented Oct 4, 2014

xref #8293

The code has evolved quite a bit and could probably benefit from refactoring.

In particular warpping the busdaycalendar logic in a wrapper class could make the code easier.

Notes about the current implementation:

  • CustomBusinessDay uses np.busdaycalendar to do the computations.
  • To speed up creation of new CustomBusinessDay offsets the calendar is cached in kwds.
  • To check equality of two CustomBusinessDay instances, the attributes holidays and weekdays are used.
  • remove EXPERIMENTAL (in docs, but check off in the experimental issue EXPM: re-evaluate experimental #5049)
  • might be some docs that say requires numpy >= 1.7 (but now all of pandas on that, so can cleanup)
@bjonen
Copy link
Contributor Author

bjonen commented Oct 4, 2014

From the previous discussion
Quoting @jreback

hmm sounds like we really should have is a wrapper object 
around np.busycalensar that is more friendly 
(and does these types of validations) and can cache itself too -
this is pretty straightforward btw

just a

class Calendar(object):

holds the actual np.busycalendar
and weekmask/holidays

@jreback jreback added Internals Related to non-user accessible pandas implementation Frequency DateOffsets Datetime Datetime data dtype Good as first PR labels Oct 4, 2014
@jreback jreback added this to the 0.15.1 milestone Oct 4, 2014
@jreback jreback changed the title Refactor pd.offsets.CustomBusinessDay CLN: Refactor pd.offsets.CustomBusinessDay Oct 4, 2014
@jreback
Copy link
Contributor

jreback commented Oct 4, 2014

@bjonen if you want to do a PR for the last point up their (any numpy 1.7 references in docs ok by me)

@TomAugspurger
Copy link
Contributor

There's been a decent amount of work on offsets recently. It's not immediately clear which of the original items are still issues. If we have specific snippets / tests for any remaining items we can reopen / open new issues.

@TomAugspurger TomAugspurger modified the milestones: Contributions Welcome, No action Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Datetime Datetime data dtype Frequency DateOffsets good first issue Internals Related to non-user accessible pandas implementation
Projects
None yet
Development

No branches or pull requests

3 participants