CLN: unify a ValueError message for removed units T, L, U, N and remove these entries from UnitChoices #59119
Conversation
natmokval
commented
Jun 27, 2024
natmokval
commented
- unify a ValueError message for removed units T, L, U, N to "Invalid frequency: T"
- removed unused entries T, L, n, u from UnitChoices
- removed a deeprecation note from the documentation of the class Timedelta
| @@ -56,10 +56,3 @@ def test_units_H_S_deprecated_from_attrname_to_abbrevs(freq): | |||
|
|
|||
| with tm.assert_produces_warning(FutureWarning, match=msg): | |||
| Resolution.get_reso_from_freqstr(freq) | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize("freq", ["T", "t", "L", "U", "N", "n"]) | |||
There was a problem hiding this comment.
Are these units tested elsewhere (that they are disallowed)?
There was a problem hiding this comment.
yes, for these units we have tests:
test_period_index_T_L_U_N_raises in pandas/tests/indexes/period/test_constructors.py
test_frequency_H_T_S_L_U_N_raises in pandas/tests/indexes/datetimes/test_date_range.py
with the error message "Invalid frequency: {}”
and with another error message "invalid unit abbreviation: {unit}"
test_timedelta_unit_T_L_U_N_raises in pandas/tests/indexes/timedeltas/test_timedelta_range.py and
test_unit_T_L_N_U_raises pandas/tests/scalar/timedelta/test_constructors.py
I am not sure, should we unify this error message to "Invalid frequency: {}”?
I removed the test_reso_abbrev_T_L_U_N_raises from pandas/tests/tslibs/test_resolution.py because we get KeyError: 'T' for this test.
There was a problem hiding this comment.
I am not sure, should we unify this error message to "Invalid frequency: {}”?
That would be nice, but if it makes the code more complex I would say it's not worth it
|
Thanks @natmokval |
