DEPR: 'A' for yearly frequency and YearEnd in favour of 'Y' by natmokval · Pull Request #55252 · pandas-dev/pandas (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

xref #54275, #54061

deprecated string 'A' for yearly frequency and YearEnd in favour of 'Y'.

EDIT:
deprecated annual frequencies with various fiscal year ends: "A-DEC", "A-JAN", etc. in favour of "Y-DEC", "Y-JAN", etc.

Choose a reason for hiding this comment

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

in offsets.pyx I think you'll need to generalise this part

                if is_period is False and name == "M":
                    warnings.warn(
                        "\'M\' will be deprecated, please use \'ME\' "
                        "for \'month end\'",
                        UserWarning,
                        stacklevel=find_stack_level(),
                    )
                    name = "ME"
                if is_period is True and name == "ME":
                    raise ValueError(
                        r"for Period, please use \'M\' "
                        "instead of \'ME\'"
                    )
                elif is_period is True and name == "M":
                    name = "ME"

in offsets.pyx I think you'll need to generalise this part

               if is_period is False and name == "M":
                   warnings.warn(
                       "\'M\' will be deprecated, please use \'ME\' "
                       "for \'month end\'",
                       UserWarning,
                       stacklevel=find_stack_level(),
                   )
                   name = "ME"
               if is_period is True and name == "ME":
                   raise ValueError(
                       r"for Period, please use \'M\' "
                       "instead of \'ME\'"
                   )
               elif is_period is True and name == "M":
                   name = "ME"

Thank you for the comment. I generalised it by adding a dictionary with frequencies deprecated for offsets.

I deprecated annual frequencies with various fiscal year ends: "A-DEC", "A-JAN", etc. in favour of "Y-DEC", "Y-JAN", etc., added tests for FutureWarning and notes in v2.2.0. @MarcoGorelli, could you please take a look at this PR?

Comment on lines 4642 to 4650

if prefix in c_DEPR_ABBREVS:
warnings.warn(
f"\'{prefix}\' is deprecated and will be removed in a "
f"future version. Please use \'{c_DEPR_ABBREVS.get(prefix)}\' "
f"instead of \'{prefix}\'.",
FutureWarning,
stacklevel=find_stack_level(),
)
prefix = c_DEPR_ABBREVS[prefix]

Choose a reason for hiding this comment

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

same as in the other PR, it's not clear to me why this moved down - could you explain please?

Choose a reason for hiding this comment

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

because I wanted to keep this together with line 4668: offset = _get_offset(prefix) where we use this prefix. There is no need to do it, I can move it back.

Choose a reason for hiding this comment

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

yeah that might be better - wouldn't we risk missing the if prefix in {"D", "H", "min", "s", "ms", "us", "ns"}: like this?

Choose a reason for hiding this comment

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

hmm, I agree, we might have problems if frequency is 'T', 'S', 'L', 'U', or 'N'. Maybe we should check
if prefix in c_DEPR_ABBREVS: before we are doing if prefix in {"D", "H", "min", "s", "ms", "us", "ns"}:
I will update my PR

Choose a reason for hiding this comment

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

I moved the check if prefix in c_DEPR_ABBREVS: back to line 4642 and added a test that fails if the order of the checks isn't correct.

DEPR: 'A' for yearly frequency and YearEnd in favour of 'Y' by natmokval · Pull Request #55252 · pandas-dev/pandas (original) (raw)

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Labels