ENH: Support for "52–53-week fiscal year" / "4–4–5 calendar" and LastWeekOfMonth DateOffset
State: closed

Closes #4511 and #4637
- Added `LastWeekOfMonth` DateOffset #4637
- Added `FY5253`, and `FY5253Quarter` DateOffsets #4511
- Improved error handling in `get_freq_code`, `_period_str_to_code` and `_base_and_stride`
- Fix issue with datetime - Timestamp


Labels:

Comments:
Right now I am using the variable `startingMonth` to mean the month in which the fiscal year ends. This seems silly but IS consistent with `BQuarterEnd`, for example.

@cancan101 Looks like u need a rebase. Can you also squash/reword most of these commits? Maybe leave 1 or 2 with a moderate amount of detail in the commit message. Thanks

@cpcloud Rebased and squashed.

@cancan101 can this be done as a parameter instead of creating new classes?

Do you mean instead of creating five new public classes and two private classes, just create 3 public classes? If so, probably, there is minimal code in two of the classes. That being said, downstream use of parameters seem clunky. See for example #5015 and some of our discussions here: https://github.com/pydata/pandas/issues/4511#issuecomment-23063826

Or do you mean using parameters instead of creating any new classes?

@cancan101 I like your `LastWeekInMonth``offset, but I think that they other 4 offsets are quite specialized. Would it not be better to have``FYear``(fiscal year), and just parameterize, with a``typ`` parameter? that would control the prefix, qtr_week_in and so on ?

You could also try an enum-like class (or toplevel defitinition) that has members that define each frequency in translation, so you'd do this:

``` python
class Offsets(object):
    LastWeekOfMonth = 'LWM'
    Day = 'D'
    Week = 'W'
    BusinessDay = 'B'
```

and then put the class in the toplevel namespace of pandas.  Not sure if it would be better, but it might help resolve this. I too don't like putting so many new classes in.

@jreback @jtratner Reducing the number of classes should not be too hard. That being said I still think it makes sense to add three classes: `LastWeekOfMonth`,  `FYear` and `FQuarter`. I feel that there is a significant difference in functionality and meaning between fiscal quarters and fiscal years. Thoughts?

Also to be clear, these new Offsets are not the standard fiscal year/ quarter but rather the "retail fiscal year" aka 52-53 week years.

bump

needs a rebase .. some changes in `offsets.py` and the usual release notes gnat

There should be release notes. Are more needed?
What changes to `offsets.py` I can reduce the number of classes, but I still think it makes sense to add three classes: `LastWeekOfMonth`, `FYear` and `FQuarter`.

I ok with those classes.

@cancan101 I meant that there were recently some changes in `offsets.py`....(now we have nanosecond `date_range`s! yay!)

@jreback I assume by those you mean the three I listed?

GIven that I am going to reduce the number of classes, what should the `rule_code` look like? It already looks ugly as is. I posted: #5015

can you put a couple of possibilites up?

@cancan101 @jreback did you mean reduce the number of classes altogether or just reduce the number of public classes?

If you combine classes, use a classmethod that takes a rulecode and spits out the correct offset.

@jreback 

For the yearly offset:
Currently: 
For last X day of month: `RL-%{month}%{day}` for example `RL-DECSAT`
For nearest X to last day of month: `RN-%{month}%{day}` for example `RN-DECSAT`

For the quarterly-offset:
For last X day of month: `RLQ-%{qtr}%{month}%{day}` for example `RLQ-2DECSAT`
For nearest X to last day of month: `RNQ-%{qtr}%{month}%{day}` for example `RNQ-1DECSAT`
where `qtr` is the quarter with the leap week.

These are hard to read since after the initial dash, there is no other delimitation. Further because of the regex I referred to in #5015, the number must be the first character after the dash.

Question: Will anything break if one `DateOffset` class produces rule codes with different prefixes?

@jtratner What do you mean by creating a "classmethod that takes a rulecode and spits out the correct offset."? Are your referring to the idea I suggested in #5028?

Given that I am going to add just `FYear` and `FQuarter`, how should the end be specified? One options would be a boolean (for example `use_closest`). Another would be an enum (which currently only takes on two values).

No, that's not what I mean.  What I mean is that you might want to let people use a class without having to pass all the arguments to the constructor.  For example, here's what we did in TimeOp so that `__init__` could take all different arguments and then return different types or None or whatever: https://github.com/pydata/pandas/blob/master/pandas/core/ops.py#L382

@jtratner I don't really follow what would be the point of that method here.

Didn't realize you only had two at this point.

@jtratner Theoretically these could be merged into `BYear` and `BQuarter`. I am hesitant to do so because there are more parameters for these new classes than apply to `BYear` and `BQuarter`.

You could then merge `BYear` and `BQuarter` into `Year` and `Quarter` and just use an enum like:

{`LastDay`, `LastBDay`, `ClosestToLastX`, `LastX`} but that seems like overkill.

Otherwise as far as my reading of Wikipedia and subsequent research has gone, there are only two variations of the 52-53 week calendar.

Sounds like you've spent a lot of time thinking about this!

@cancan101 can you rebase and update?

@jreback Can I get some feedback on the rule code?

@cancan101 excellent effort!

i'm not familiar with the rules here so i defer to someone who is.

re line length
a good rule of thumb about line length is: if you have to scroll in the github diff it's too long

@cancan101 I think I liked your solution with `LastInMonth`, `FYear`, `FQuarter` as public classes only. What was the other question?

Will there be problems if the same class has different "prefixes" (the
first part before the hyphen) in the rule code?
On Oct 4, 2013 7:18 PM, "jreback" notifications@github.com wrote:

> @cancan101 https://github.com/cancan101 I think I liked your solution
> with LastInMonth, FYear, FQuarter as public classes only. What was the
> other question?
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/pydata/pandas/pull/5004#issuecomment-25736480
> .

One remaining question @jreback @jtratner @cpcloud:

Given that I am going to add just FYear and FQuarter (and LastInMonth), how should the end be specified? One options would be a boolean (for example `use_closest`). Another would be an enum (which currently only takes on two values).