clean: simplify lite-rule-alias and dont-uppercase #59240
Conversation
MarcoGorelli
force-pushed
the
lite-rule-alias
branch
from
bf818b2 to
5a0ea7a
Compare
| _dont_uppercase = {"h", "bh", "cbh", "MS", "ms", "s"} | ||
| _dont_uppercase = {"min", "h", "bh", "cbh", "s", "ms", "us", "ns"} |
There was a problem hiding this comment.
It never made much sense to me that 'MS' was in this list. It's been there since forever, but it looks odd - 'MS' is already uppercase 😄
There was a problem hiding this comment.
agree, it was always confusing for me.
MarcoGorelli
force-pushed
the
lite-rule-alias
branch
from
a5e0c13 to
359888e
Compare
MarcoGorelli
force-pushed
the
lite-rule-alias
branch
from
359888e to
317c3ae
Compare
| @pytest.mark.parametrize("freq_depr", ["2MIN", "2mS", "2Us"]) | ||
| @pytest.mark.parametrize("freq_depr", ["2MIN", "2nS", "2Us"]) |
There was a problem hiding this comment.
With this PR, "2mS" would just raise a hard error. I think that's OK tbh, anyone using 'ms' case-insentively was already playing with fire
| ("b", BDay()), | ||
| ("bme", BMonthEnd()), | ||
| ("Bme", BMonthEnd()), | ||
| ("BME", BMonthEnd()), |
There was a problem hiding this comment.
'b' raises a deprecation warning (and it's already tested, e.g. test_construction_bday and test_construction_bday
'bme' would raise a hard error, but I think that's fine too, as 'me' already went through a deprecation cycle
There was a problem hiding this comment.
yes, seems reasonable to me
|
@MarcoGorelli, these changes look good to me. |
MarcoGorelli
changed the title
|
Thanks @MarcoGorelli |
work in progress, trying to clean this confusing code
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.