DEPR: Period[B]

[jbrockmendel]

Pandas version checks

• I have checked that this issue has not already been reported.

• I have confirmed this bug exists on the latest version of pandas.

• I have confirmed this bug exists on the main branch of pandas.

Reproducible Example

NA

Issue Description

TL;DR: can we deprecate BDay Period(|Dtype|Index|Array) and tell users to use a DatetimeIndex with freq="B" instead?

Context: lots of user confusion has resulted from a) using "freq" for PeriodX to mean something different than it does for DatetimeIndex (#51256, #38885, #47227) and b) using DateOffsets to define PeriodX (#5091, #13871, #38914, #38859).

The option I'm leaning toward to address this is to change/deprecate PeriodX.freq to "unit" (the attribute, the constructor keywords, and asfreq->as_unit). The sticking point I'm finding is in finding a replacement for the current usage period + period.freq * N. For everything except BDay, we could tell users to do something like period + np.timedelta64(N, period.unit) instead (actually just period + N works, but I'd actually prefer to deprecate that).

So I'm wondering if we really need to support Period[B]. Anyone have a strong opinion?

Expected Behavior

NA

Installed Versions

Replace this line with the output of pd.show_versions()

[MarcoGorelli]

Reproducible Example
NA

Any chance you could include an example of what you'd like to deprecate, and what users should do instead please?

[jbrockmendel]

Any chance you could include an example of what you'd like to deprecate, and what users should do instead please?

pd.anything(..., dtype="Period[B]")
pd.period_range(..., freq="B")
pd.PeriodIndex(..., freq="B")
pd.Period(..., freq="B")

For the PeriodIndex cases I'd advise users to use a DatetimeIndex with freq="B". For the a user would be out of luck.

[MarcoGorelli]

sounds fine, thanks!

[torext]

This now seems to be causing a lot of warnings when simply calling .plot() on a DataFrame with freq="B" DatetimeIndex:

Failed to convert value(s) to axis units: (datetime.date(1970, 1, 1), datetime.date(1970, 1, 2))
  File "/home/torext/miniconda3/envs/testenv/lib/python3.11/site-packages/matplotlib/axis.py", line 1732, in convert_units
    ret = self.converter.convert(x, self.units, self)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/torext/miniconda3/envs/testenv/lib/python3.11/site-packages/pandas/plotting/_matplotlib/converter.py", line 238, in convert
    values = PeriodConverter._convert_1d(values, units, axis)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/torext/miniconda3/envs/testenv/lib/python3.11/site-packages/pandas/plotting/_matplotlib/converter.py", line 257, in _convert_1d
    return [get_datevalue(x, axis.freq) for x in values]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/torext/miniconda3/envs/testenv/lib/python3.11/site-packages/pandas/plotting/_matplotlib/converter.py", line 257, in <listcomp>
    return [get_datevalue(x, axis.freq) for x in values]
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/torext/miniconda3/envs/testenv/lib/python3.11/site-packages/pandas/plotting/_matplotlib/converter.py", line 265, in get_datevalue
    return Period(date, freq).ordinal
           ^^^^^^^^^^^^^^^^^^
  File "/home/torext/miniconda3/envs/testenv/lib/python3.11/site-packages/period.pyx", line 2823, in pandas._libs.tslibs.period.Period.__new__
FutureWarning: Period with BDay freq is deprecated and will be removed in a future version. Use a DatetimeIndex with BDay freq instead.

The above exception was the direct cause of the following exception:

Any chance this can be fixed? I (guess I) understand deprecating this use of the Period object, but if it mostly just affects internals, which when called normally by the user work but now throw a bunch of warnings the user can do nothing about, then it's quite annoying...

[mroeschke]

@jbrockmendel do you remember if there were any challenges with silencing this warnings for the plotting code?

[torext]

Also, what's curious is that running my script throws with

FutureWarning: PeriodDtype[B] is deprecated and will be removed in a future version. Use a DatetimeIndex with freq='B' instead

even though I added the following at the very start of my entry point:

warnings.simplefilter("error")
warnings.filterwarnings("ignore", "Period with BDay freq is deprecated", FutureWarning)
warnings.filterwarnings(
    "ignore", "PeriodDtype[B] is deprecated and will be removed", FutureWarning
)

I.e. that second filter I added does override the very first simplefilter, but the very last filter has no effect.

[jbrockmendel]

@jbrockmendel do you remember if there were any challenges with silencing this warnings for the plotting code?

IIIRC it was as normal filterwarnings without any complications. There's a note-to-self that we need a better solution before the deprecation is enforced.

[torext]

A very simple repro showing the warnings happening is as follows:

import warnings
import pandas._testing as tm

warnings.simplefilter("error")
_ = tm.makeTimeDataFrame().plot()

For me this throws even with the latest checkout of the repo. In fact, the first test in pandas/tests/plotting/frame/test_frame.py shows the same warnings.

There's a note-to-self that we need a better solution before the deprecation is enforced.

Stepping through the plotting code it looks to me like that place where this note was put is not sufficient to capture all instances of the warning: it silences the warning that would happen on line 1363 of pandas/plotting/_matplotlib/core.py, but then the warning gets triggered again on line 1392 of the same module.

Given the warning should be suppressed for the most naive and simple use of plotting functionality, may I suggest the warning filter get moved way higher up the hierarchy, potentially all the way to the __call__ method of the PlotAccessor even? That way any more specialised/direct use of the Period objects doesn't get the warnings filtered, but simple users like me aren't left scratching their head 🙂

[jbrockmendel]

may I suggest the warning filter get moved way higher up the hierarchy

I'd be open to that, but first I'd like to try getting the warning suppression closer to the source of the problem. I can get the example you posted to run by catching warnings in PeriodConverter.convert_1d and TimeSeries_DateFormatter.__call__. I suspect catching in _daily_finder might also be necessary for some other cases.

PR would be welcome.

[torext]

PR would be welcome.

@jbrockmendel I submitted a PR based on your suggestions: #55138. I still feel that a single filter at the level of, say, PlotAccessor.__call__ would be more conservative and would have the advantage of just one single location where the filter is placed, as opposed to now having these multiple (I think 4?) invocations of what is essentially the same filter.