Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 22, 2013
+945 −55
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion doc/source/release.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ New features
- Auto-detect field widths in read_fwf when unspecified (:issue:`4488`)
- ``to_csv()`` now outputs datetime objects according to a specified format string
via the ``date_format`` keyword (:issue:`4313`)

- Added ``LastWeekOfMonth`` DateOffset (:issue:`4637`)
- Added ``FY5253``, and ``FY5253Quarter`` DateOffsets (:issue:`4511`)

Experimental Features
~~~~~~~~~~~~~~~~~~~~~
Expand Down
3 changes: 3 additions & 0 deletions doc/source/timeseries.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@ frequency increment. Specific offset logic like "month", "business day", or
CDay, "custom business day (experimental)"
Week, "one week, optionally anchored on a day of the week"
WeekOfMonth, "the x-th day of the y-th week of each month"
LastWeekOfMonth, "the x-th day of the last week of each month"
MonthEnd, "calendar month end"
MonthBegin, "calendar month begin"
BMonthEnd, "business month end"
Expand All @@ -428,10 +429,12 @@ frequency increment. Specific offset logic like "month", "business day", or
QuarterBegin, "calendar quarter begin"
BQuarterEnd, "business quarter end"
BQuarterBegin, "business quarter begin"
FY5253Quarter, "retail (aka 52-53 week) quarter"
YearEnd, "calendar year end"
YearBegin, "calendar year begin"
BYearEnd, "business year end"
BYearBegin, "business year begin"
FY5253, "retail (aka 52-53 week) year"
Hour, "one hour"
Minute, "one minute"
Second, "one second"
Expand Down
2 changes: 2 additions & 0 deletions doc/source/v0.13.0.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,8 @@ Enhancements
- Python csv parser now supports usecols (:issue:`4335`)

- DataFrame has a new ``interpolate`` method, similar to Series (:issue:`4434`, :issue:`1892`)
- Added ``LastWeekOfMonth`` DateOffset (:issue:`4637`)
- Added ``FY5253``, and ``FY5253Quarter`` DateOffsets (:issue:`4511`)


.. ipython:: python
Expand Down
46 changes: 27 additions & 19 deletions pandas/tseries/frequencies.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ def get_freq_code(freqstr):
code = _period_str_to_code(freqstr[0])
stride = freqstr[1]
except:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpcloud I am thinking that this should also be changed to raise ValueError rather than KeyError

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NM. That is already handled in _period_str_to_code

if com.is_integer(freqstr[1]):
raise
code = _period_str_to_code(freqstr[1])
stride = freqstr[0]
return code, stride
Expand Down Expand Up @@ -227,10 +229,10 @@ def get_period_alias(offset_str):
'us': 'U'
}

#TODO: Can this be killed?
for _i, _weekday in enumerate(['MON', 'TUE', 'WED', 'THU', 'FRI']):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just define a no argument function (add add global declarations where necessary) and call it. then you don't need to have all these underscored variables (obviously keep the ones that were there before)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpcloud What do you think about moving this logic to a classmethod on the respective offsets?
Also I am not quite sure what you mean by the function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

classmethod probably ok....

by function i just meant something like

def initializer():
    # your init code here
initializer()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea with classmethods is to put the logic for generating all instances of a given offset with the offset itself rather than having the code in separate files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initializer for now

for _iweek in range(4):
_name = 'WOM-%d%s' % (_iweek + 1, _weekday)
_offset_map[_name] = offsets.WeekOfMonth(week=_iweek, weekday=_i)
_rule_aliases[_name.replace('-', '@')] = _name

# Note that _rule_aliases is not 1:1 (d[BA]==d[A@DEC]), and so traversal
Expand Down Expand Up @@ -301,7 +303,7 @@ def to_offset(freqstr):


# hack to handle WOM-1MON
opattern = re.compile(r'([\-]?\d*)\s*([A-Za-z]+([\-@]\d*[A-Za-z]+)?)')
opattern = re.compile(r'([\-]?\d*)\s*([A-Za-z]+([\-@][\dA-Za-z\-]+)?)')


def _base_and_stride(freqstr):
Expand Down Expand Up @@ -356,16 +358,16 @@ def get_offset(name):
else:
if name in _rule_aliases:
name = _rule_aliases[name]
try:
if name not in _offset_map:

if name not in _offset_map:
try:
# generate and cache offset
offset = _make_offset(name)
_offset_map[name] = offset
return _offset_map[name]
except (ValueError, TypeError, KeyError):
# bad prefix or suffix
pass
raise ValueError('Bad rule name requested: %s.' % name)
except (ValueError, TypeError, KeyError):
# bad prefix or suffix
raise ValueError('Bad rule name requested: %s.' % name)
_offset_map[name] = offset
return _offset_map[name]


getOffset = get_offset
Expand Down Expand Up @@ -401,9 +403,6 @@ def get_legacy_offset_name(offset):
name = offset.name
return _legacy_reverse_map.get(name, name)

get_offset_name = get_offset_name


def get_standard_freq(freq):
"""
Return the standardized frequency string
Expand Down Expand Up @@ -621,8 +620,12 @@ def _period_str_to_code(freqstr):
try:
freqstr = freqstr.upper()
return _period_code_map[freqstr]
except:
alias = _period_alias_dict[freqstr]
except KeyError:
try:
alias = _period_alias_dict[freqstr]
except KeyError:
raise ValueError("Unknown freqstr: %s" % freqstr)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpcloud is it acceptable to change the exception from KeyError to ValueError? I will have to change the unit test for this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure ... that makes a lot more sense. a user doesn't care whether the period aliases are stored in a dict, all they care about is that they passed an invalid freqstr value, so ValueError is much more appropritae


return _period_code_map[alias]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you catch the KeyError here as well and provide a more informative error message? it's slightly annoying when you pass a frequency that isn't defined and you get a KeyError. there is a test for this, so it's acknowledged that it does raise but it's not very user-friendly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.



Expand Down Expand Up @@ -839,16 +842,21 @@ def _get_monthly_rule(self):
'ce': 'M', 'be': 'BM'}.get(pos_check)

def _get_wom_rule(self):
wdiffs = unique(np.diff(self.index.week))
if not lib.ismember(wdiffs, set([4, 5])).all():
return None
# wdiffs = unique(np.diff(self.index.week))
#We also need -47, -49, -48 to catch index spanning year boundary
# if not lib.ismember(wdiffs, set([4, 5, -47, -49, -48])).all():
# return None

weekdays = unique(self.index.weekday)
if len(weekdays) > 1:
return None

week_of_months = unique((self.index.day - 1) // 7)
if len(week_of_months) > 1:
return None

# get which week
week = (self.index[0].day - 1) // 7 + 1
week = week_of_months[0] + 1
wd = _weekday_rule_aliases[weekdays[0]]

return 'WOM-%d%s' % (week, wd)
Expand Down
Loading