BUG: renaming offsets quarterly frequencies with various fiscal year ends #55711
Conversation
…ont_uppercase, fix test
MarcoGorelli left a comment
MarcoGorelli left a comment There was a problem hiding this comment.
Nice, thanks! Just got a question about something, not sure why it's necessary
pandas/_libs/tslibs/offsets.pyx Outdated
| if name.upper() != name: | ||
| name = name.lower() |
There was a problem hiding this comment.
it's necessary in order to catch frequencies like 'qe-MAr' with mix of lower/uppercase letters. Otherwise we should add all possible combinations to _dont_uppercase list.
There was a problem hiding this comment.
ah I see, thanks!
Before this change:
In [4]: _get_offset('Qe-mAR') Out[4]: <QuarterEnd: startingMonth=3>After this change:
In [2]: _get_offset('Qe-mAR') --------------------------------------------------------------------------- ValueError: Invalid frequency: qe-marWould it work to simplify and just do
- if name.upper() != name: - name = name.lower() - if name not in _dont_uppercase: + if name.lower() not in _dont_uppercase:?
There was a problem hiding this comment.
Thank you, I simplified the condition. I just wasn't sure that the frequency like ‘QE-MAR’ should also pass this check.
It looks confusing: we take an uppercase string, then lowercase it and check if the string is in the _dont_uppercase list.
There was a problem hiding this comment.
agree, but I think we want to get rid of _dont_uppercase completely anyway
| [ | ||
| ("y-dec", "<YearEnd: month=12>"), | ||
| ("qe-mar", "<QuarterEnd: startingMonth=3>"), | ||
| ("Q-MAR", "<QuarterEnd: startingMonth=3>"), |
There was a problem hiding this comment.
nice - this is a period, so indeed the period alias should be used
2 participants
xref #55553
'qe-dec','qe-jan',and other lowercase strings for offsets quarterly frequencies with various fiscal year ends are added to _dont_uppercase list. The reason: for period they are silently converted to'Q-DEC', etc.test
test_not_subperiodis fixed.