API: add is_beg_month/quarter/year, is_end_month/quarter/year accessors (#4565)
State: closed

closes #4565


Labels:
name: Enhancement
name: Datetime
description: Datetime data dtype
name: Frequency
description: DateOffsets

Comments:
We should add these accessors to the DatetimeIndex documentation I started with #4706

sure...you can add some docs on this (and for all accessors like this for that matter)

All the other accessors are already there.  I can add these.

I don't think the quarter accessors are well defined. There can be various quarterly cycles other than the one in this commit.

Is this a bug?

```
from pandas.tseries.frequencies import get_offset
get_offset('Q').freqstr
'Q-DEC'
get_offset('QS').freqstr
'QS-JAN'
```

I don't think so. One is quarterly periods defined by year ending in
December and the other is quarterly periods defined by year starting in
January.
On Sep 18, 2013 3:38 AM, "Michael Bailey" notifications@github.com wrote:

> Is this a bug?
> 
> from pandas.tseries.frequencies import get_offset
> get_offset('Q').freqstr
> 'Q-DEC'
> get_offset('QS').freqstr
> 'QS-JAN'
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/4823#issuecomment-24646338
> .

In the docs it says:

(B)Q(S)-JAN quarterly frequency, year ends in January

So that is incorrect then, Q-JAN is different form QS-JAN in terms of ending year. 

I was also thrown by the fact that QuarterBegin() and QuarterEnd() share the same keyword 'startingMonth' and assumed they both referenced the last month of the year. It also seems odd that we instantiate QuarterBegin() with startingMonth = 3? 

x=pd.Timestamp('2012-01-01', offset='QS')
y=pd.Timestamp('2012-03-01', offset='QS')
print pd.tseries.offsets.QuarterBegin().onOffset(x)
False
print pd.tseries.offsets.QuarterBegin().onOffset(y)
True

Is there a reason we don't have 'staringMonth' and 'endingMonth' offsets to distinguish them? Or even better, use the 'endingMonth' offset for all offsets with end months. 

@mikebailey I think you pointed out some related bugs here...see #4069, #4109

@mikebailey see also #2885 

ping!

@jreback, I've looked into the bugs you've posted, I can wrap these all up in this PR without too much trouble I believe - I'll be off the grid for a couple of weeks and probably won't be able to finish before I leave, but will take this on when I get back. 

gr8

pushing to 0.14

@jreback @cancan101 working on these offset bugs now. What are your thoughts on changing the default behavior of these offsets? Ideally all quarter and annual offsets would default to beginning jan 1st - but that would break backwards compatibility, should I forge ahead with the change? 

I also want to unify on a single `month` keyword rather than having some with `startingMonth` and others using `month` which is confusing. My preference would be deprecate the `startingMonth` keyword and have `month` always mean the ending month for the period. We could also implement `startingMonth` and `endingMonth` keywords for all relevant period offsets to avoid confusion and pick a default way to resolve `month`. thoughts?

@mikebailey first pls rebase rebase on master, a big PR by @cancan101 was recently merged which add some new offset stuff (and @jtratner did a lot of cleaning on how the offsets are created).

I believe @cancan101 has some thoughts on the naming scheme internal to offsets?

we don't want to break back compat unless have a really good reason.

@cancan101 was suggesting the same thing about changing the month kwarg
name. My preference is to push until the next release so we can improve the
documentation of DateOffset at the same time that we change its API.

It's trivial to make this change because all the constructors are single
positional + keyword argument. This should be clearer after you rebase.

Either you or @cancan101 should make the change, but it needs to be in a
separate PR from this one.

@mikebailey I agree that the `startingMonth` keyword is both misleading and actually wrong (see #5307). I am all for consistency. In this case I would value consistency with industry convention over consistency within Pandas. What I mean by that is not changing all periods to be specified based on when they end by either by what would make sense or what is an industry norm. See here #4591.

I would vote to having periods like `YearEnd` and `QuarterEnd` (and the new `FY5253`)  having an `endingMonth` param and periods like `YearStart` keeping the  `startingMonth`. I would not change everything to `month` since that is ambiguous and not self-documenting. 

I definitely would not change existing the default behavior.

Would it be possible to just define a function that dispatches based upon
arguments, instead of redefining the constructors? Particularly in terms of
backwards compatibility, I'd rather add a new function that translates user
arguments into appropriate offsets than change the constructors for ten
different objects.  Eventually we could deprecate direct access to the
existing offset classes. It's just much more awkward to maintain so many
constructors.

@jtratner I think thanks to your refactoring there are far fewer constructors than in the past.

@cancan101 there are fewer `__init__` methods, but still a bunch of different entry points.

Btw - really trivial to change startingMonth if you think it's necessary, just make sure you accept but warn if startingMonth is passed to the constructor, and put in one really quick test for each class to check that it's actually warning.

@mikebailey Any interest in tackling that?

Interest is high, but feel free to do this if you want it done sooner rather than later. I'll first fix up this PR with the recent changes. 

So it seems like the consensus is to keep the startingMonth and endingMonth keywords and default behavior, but fix them so that endingMonth means the ending month and starting month means the starting month. Ideally this logic would be pushed into a function instead of separately into each constructor.  

I still need to make this work for FY5253 changes - if I get ambitious enough I'll tackle custom business dates as well. 

@mikebailey can you review this and address some of the above comments....thanks

@mikebailey you will need to refactor this a bit after I merge #6382 (and pay special attention that these _could_ also be defined for PeriodIndex too)

@mikebailey still working on this?

@jreback, yes I am. I will get a diff in this week. Thanks for your
patience.

On Sun, Mar 9, 2014 at 7:58 AM, jreback notifications@github.com wrote:

> @mikebailey https://github.com/mikebailey still working on this?
> 
> ## 
> 
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/4823#issuecomment-37128739
> .

## 

mikebailey

@mikebailey if you can rebase would be gr8

@jreback 1) git question, I rebased using:
git fetch upstream 
git rebase upstream/master

and resolving conflicts. When I try to push it wants me to do a git pull: 
https://gist.github.com/mikebailey/9720071

This doesn't seem like a good idea as it would put all these commits into this PR? What should I do? I just did a git push --force for now. 

2) Currently unable to test because of this error when trying to import pandas:

https://gist.github.com/mikebailey/9720049

I am building the extensions in place. I don't get this error when I try to rebuild off of the stable version of pandas so it must be local to this PR. I am guessing it is something to do with tslib.pyx. Any ideas on where I could get help for resolving this? 

you have to force push
use the -f flag

`git push myfork mybranch -f`

since u changed cython code u need to rebuild the extensions

easiest to just do: make

which is equivalent to

`python setup build_ext --inplace`