CLN: enforced the deprecation of strings 'H', 'BH', 'CBH'  in favor of 'h', 'bh', 'cbh'
State: closed

Enforced deprecation of uppercase strings ‘H’, ‘BH’, ‘CBH’ for offsets frequencies, resolution abbreviations, and units  in favor of 'h', 'bh', 'cbh'


Labels:
name: Clean

Comments:
> thanks for your PR!
> 
> Is it necessary to introduce `c_REMOVED_ABBREVS`? I tried just removing it altogether, i.e.
> 
> ```diff
> diff --git a/pandas/_libs/tslibs/dtypes.pxd b/pandas/_libs/tslibs/dtypes.pxd
> index 1ceb837d39..d8c536a34b 100644
> --- a/pandas/_libs/tslibs/dtypes.pxd
> +++ b/pandas/_libs/tslibs/dtypes.pxd
> @@ -14,7 +14,6 @@ cdef bint is_supported_unit(NPY_DATETIMEUNIT reso)
>  cdef dict c_OFFSET_TO_PERIOD_FREQSTR
>  cdef dict c_PERIOD_TO_OFFSET_FREQSTR
>  cdef dict c_OFFSET_RENAMED_FREQSTR
> -cdef dict c_REMOVED_ABBREVS
>  cdef dict c_DEPR_UNITS
>  cdef dict c_PERIOD_AND_OFFSET_DEPR_FREQSTR
>  cdef dict attrname_to_abbrevs
> diff --git a/pandas/_libs/tslibs/dtypes.pyx b/pandas/_libs/tslibs/dtypes.pyx
> index 08a398f7a3..f1cb5ce9fb 100644
> --- a/pandas/_libs/tslibs/dtypes.pyx
> +++ b/pandas/_libs/tslibs/dtypes.pyx
> @@ -335,14 +335,6 @@ PERIOD_TO_OFFSET_FREQSTR = {
>  cdef dict c_OFFSET_TO_PERIOD_FREQSTR = OFFSET_TO_PERIOD_FREQSTR
>  cdef dict c_PERIOD_TO_OFFSET_FREQSTR = PERIOD_TO_OFFSET_FREQSTR
>  
> -# Map deprecated resolution abbreviations to correct resolution abbreviations
> -cdef dict c_REMOVED_ABBREVS = {
> -    "H": "h",
> -    "BH": "bh",
> -    "CBH": "cbh",
> -    "S": "s",
> -}
> -
>  cdef dict c_DEPR_UNITS = {
>      "w": "W",
>      "d": "D",
> @@ -451,9 +443,7 @@ class Resolution(Enum):
>          True
>          """
>          try:
> -            if freq in c_REMOVED_ABBREVS:
>  
> -                raise ValueError(INVALID_FREQ_ERR_MSG.format(freq))
>              attr_name = _abbrev_to_attrnames[freq]
>          except KeyError:
>              # For quarterly and yearly resolutions, we need to chop off
> @@ -464,8 +454,6 @@ class Resolution(Enum):
>              if split_freq[1] not in _month_names:
>                  # i.e. we want e.g. "Q-DEC", not "Q-INVALID"
>                  raise
> -            if split_freq[0] in c_REMOVED_ABBREVS:
> -                raise ValueError(INVALID_FREQ_ERR_MSG.format(split_freq[0]))
>              attr_name = _abbrev_to_attrnames[split_freq[0]]
>  
>          return cls.from_attrname(attr_name)
> diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx
> index a30d738cc1..6e17f8f6ae 100644
> --- a/pandas/_libs/tslibs/offsets.pyx
> +++ b/pandas/_libs/tslibs/offsets.pyx
> @@ -60,7 +60,6 @@ from pandas._libs.tslibs.dtypes cimport (
>      c_OFFSET_TO_PERIOD_FREQSTR,
>      c_PERIOD_AND_OFFSET_DEPR_FREQSTR,
>      c_PERIOD_TO_OFFSET_FREQSTR,
> -    c_REMOVED_ABBREVS,
>      periods_per_day,
>  )
>  from pandas._libs.tslibs.nattype cimport (
> @@ -4908,8 +4907,6 @@ cpdef to_offset(freq, bint is_period=False):
>                  if not stride:
>                      stride = 1
>  
> -                if prefix in c_REMOVED_ABBREVS:
> -                    raise ValueError(INVALID_FREQ_ERR_MSG.format(prefix))
>  
>                  if prefix in {"D", "h", "min", "s", "ms", "us", "ns"}:
>                      # For these prefixes, we have something like "3h" or
> ```
> 
> and it seems fine, did I miss something?
> 
thanks, @MarcoGorelli for helping me with this PR. You are right, there is no need to have `c_REMOVED_ABBREVS`. I removed it altogether.
 > In
> 
> https://github.com/pandas-dev/pandas/blob/7d2980cf62e6bde22997b855b9c6b6adf28c6b70/pandas/_libs/tslibs/dtypes.pyx#L447-L457
> 
> you may also want to change `raise` to something like `raise ValueError(err_msg) from exc`, where `exc` is the `KeyError`?
>
thanks, I did as you suggested and replaced `raise` with `raise ValueError(msg) from exc`, where `exc` is the `KeyError`, e.g.
```
except KeyError as exc:
    msg = INVALID_FREQ_ERR_MSG.format(freq)
    # For quarterly and yearly resolutions, we need to chop off
    #  a month string.
    split_freq = freq.split("-")
    if len(split_freq) != 2:
        raise ValueError(msg) from exc
```
Thanks @natmokval