simplify+unify offset.apply logic
State: closed

Implement _get_lastbday mirroring _get_firstbday.

Implement bstart and bend cases in shift month.  Use these to simplify a bunch of the BusinessFoo offsets.


Labels:
name: Frequency
description: DateOffsets
name: Clean

Comments:
Running asv now.
```
taskset 6 asv continuous -f 1.1 -E virtualenv master HEAD -b timeseries -b timedelta -b period
[...]
       before           after         ratio
     [7f4c960c]       [f6aebe37]
+           44.3s            1.07m     1.45  gil.nogil_datetime_fields.time_period_to_datetime
+           15.0s            17.1s     1.14  gil.nogil_datetime_fields.time_datetime_to_period
+         760±2ns          851±5ns     1.12  period.Properties.time_daysinmonth
-      30.6±0.3μs       27.8±0.2μs     0.91  timeseries.Offsets.time_custom_bday_decr
-     1.87±0.06ms      1.66±0.01ms     0.89  timeseries.DatetimeIndex.time_add_timedelta
-         811±5ns          720±4ns     0.89  period.Properties.time_day
-         836±8ns          736±3ns     0.88  period.PeriodProperties.time_hour
-         204±2ns        154±0.1ns     0.76  timedelta.TimedeltaProperties.time_timedelta_days
-        191±10ms       27.2±0.2ms     0.14  timeseries.AsOfDataFrame.time_asof_nan
-        202±10ms       24.2±0.3ms     0.12  timeseries.AsOfDataFrame.time_asof

taskset 6 asv continuous -f 1.1 -E virtualenv master HEAD -b timeseries -b timedelta -b period
[...]
       before           after         ratio
     [7f4c960c]       [f6aebe37]
+      15.8±0.6μs       20.2±0.1μs     1.28  timeseries.Offsets.time_custom_bday_apply
+      27.1±0.2μs       32.8±0.1μs     1.21  timeseries.Offsets.time_custom_bday_cal_decr
+         725±5ns          833±9ns     1.15  period.PeriodProperties.time_minute
+     16.7±0.08μs       19.0±0.1μs     1.14  timeseries.AsOf.time_asof_single_early
+      17.9±0.1μs       20.3±0.3μs     1.13  timeseries.Offsets.time_timeseries_day_apply
+         102±2μs          114±1μs     1.12  timeseries.Offsets.time_custom_bmonthend_decr_n
+       134±0.9ms          150±1ms     1.12  timeseries.ToDatetime.time_iso8601_tz_spaceformat
+     18.9±0.06μs       21.0±0.2μs     1.11  timeseries.SemiMonthOffset.time_end_incr_n
-           44.6s            39.6s     0.89  gil.nogil_datetime_fields.time_period_to_datetime
-      20.3±0.1μs       17.0±0.1μs     0.84  timeseries.Offsets.time_custom_bday_apply_dt64
-        28.3±2ms           15.7ms     0.56  timeseries.DatetimeIndex.time_dti_tz_factorize
-        28.1±1ms       6.93±0.6ms     0.25  timeseries.DatetimeIndex.time_dti_factorize

taskset 6 asv continuous -f 1.1 -E virtualenv master HEAD -b timeseries -b timedelta -b period
[...]
       before           after         ratio
     [7f4c960c]       [f6aebe37]
+           14.5s            17.0s     1.17  gil.nogil_datetime_fields.time_datetime_to_period
-      8.14±0.5ms      7.00±0.05ms     0.86  timeseries.DatetimeIndex.time_add_offset_fast
-        29.1±1ms       6.95±0.5ms     0.24  timeseries.DatetimeIndex.time_dti_tz_factorize
-        33.2±5ms       6.44±0.4ms     0.19  timeseries.DatetimeIndex.time_dti_factorize
-         301±7ms         57.2±1ms     0.19  timeseries.AsOfDataFrame.time_asof
-         318±9ms       59.7±0.6ms     0.19  timeseries.AsOfDataFrame.time_asof_nan

taskset 6 asv continuous -f 1.1 -E virtualenv master HEAD -b timeseries -b timedelta -b period
[...]
      before           after         ratio
     [7f4c960c]       [f6aebe37]
+     12.0±0.04μs      13.4±0.08μs     1.12  timeseries.SemiMonthOffset.time_begin_apply
+      10.5±0.1μs      11.6±0.02μs     1.11  timeseries.Offsets.time_timeseries_year_incr
-      28.0±0.1μs      24.1±0.07μs     0.86  timeseries.Offsets.time_custom_bday_cal_incr
-      29.5±0.6ms       6.74±0.3ms     0.23  timeseries.DatetimeIndex.time_dti_tz_factorize

```

The optimist in me thinks the time_dti_tz_factorize might be real, but for the most part this looks like a wash.
```
       before           after         ratio
     [7f4c960c]       [f6aebe37]
+      46.2±0.4ms       51.8±0.6ms     1.12  timedelta.ToTimedelta.time_convert_string
-           2.37s            2.02s     0.85  timeseries.AsOfDataFrame.time_asof_nan
-      18.4±0.1μs      15.6±0.05μs     0.85  timeseries.SemiMonthOffset.time_end_incr
-       170±0.4ms        138±0.6ms     0.81  timeseries.ToDatetime.time_iso8601_tz_spaceformat
```
These numbers are against an out-of-date master.  Will update. <b>update</b> well maybe not _that_ out of date.
Making edits per comments now.  As to test coverage (here and in the other two similar PRs): as I went through these I added equality assertions for new vs old versions followed by `assert False` to make sure they were actually hit.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=h1) Report
> Merging [#18263](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/77f10f0805651493b95a462085c9a3cd98e9197a?src=pr&el=desc) will **decrease** coverage by `51.95%`.
> The diff coverage is `0%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/18263/graphs/tree.svg?src=pr&width=650&token=eZ4WkYLtcO&height=150)](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree)

```diff
@@             Coverage Diff             @@
##           master   #18263       +/-   ##
===========================================
- Coverage    91.4%   39.44%   -51.96%     
===========================================
  Files         164      164               
  Lines       49880    49857       -23     
===========================================
- Hits        45592    19667    -25925     
- Misses       4288    30190    +25902
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `?` | |
| #single | `39.44% <0%> (-0.04%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/tseries/offsets.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvb2Zmc2V0cy5weQ==) | `34.33% <0%> (-62.78%)` | :arrow_down: |
| [pandas/io/formats/style.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvc3R5bGUucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/tools/hashing.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL3Rvb2xzL2hhc2hpbmcucHk=) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/types/concat.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL3R5cGVzL2NvbmNhdC5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/io/sas/sas\_constants.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2lvL3Nhcy9zYXNfY29uc3RhbnRzLnB5) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/io/json/json.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2lvL2pzb24vanNvbi5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/parser.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL3BhcnNlci5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/lib.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2xpYi5weQ==) | `0% <0%> (-100%)` | :arrow_down: |
| [pandas/io/sas/sas7bdat.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2lvL3Nhcy9zYXM3YmRhdC5weQ==) | `0% <0%> (-91.08%)` | :arrow_down: |
| [pandas/io/sas/sas\_xport.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2lvL3Nhcy9zYXNfeHBvcnQucHk=) | `0% <0%> (-90.28%)` | :arrow_down: |
| ... and [114 more](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=footer). Last update [77f10f0...0b1ecaf](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=h1) Report
> Merging [#18263](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/9c799e2c4331e5e42cfda03323eef165feb5be1a?src=pr&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/18263/graphs/tree.svg?width=650&height=150&src=pr&token=eZ4WkYLtcO)](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #18263      +/-   ##
==========================================
- Coverage   91.39%   91.36%   -0.04%     
==========================================
  Files         164      164              
  Lines       49854    49781      -73     
==========================================
- Hits        45566    45484      -82     
- Misses       4288     4297       +9
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.17% <100%> (-0.02%)` | :arrow_down: |
| #single | `39.49% <0%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/tseries/offsets.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvb2Zmc2V0cy5weQ==) | `96.91% <100%> (-0.16%)` | :arrow_down: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.8% <0%> (-0.1%)` | :arrow_down: |
| [pandas/plotting/\_core.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb3JlLnB5) | `82.45% <0%> (ø)` | :arrow_up: |
| [pandas/core/reshape/reshape.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9yZXNoYXBlLnB5) | `100% <0%> (ø)` | :arrow_up: |
| [pandas/core/indexes/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9wZXJpb2QucHk=) | `92.9% <0%> (ø)` | :arrow_up: |
| [pandas/core/indexes/numeric.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9udW1lcmljLnB5) | `97.27% <0%> (ø)` | :arrow_up: |
| [pandas/core/indexes/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy90aW1lZGVsdGFzLnB5) | `91.14% <0%> (ø)` | :arrow_up: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.48% <0%> (ø)` | :arrow_up: |
| ... and [6 more](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=footer). Last update [9c799e2...45d9ddd](https://codecov.io/gh/pandas-dev/pandas/pull/18263?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

> also can you unit test some these?

Yes, but since I just added a test_liboffsets file in #18280 I'd rather close these out separately and then add unit tests in a shared follow-up.
@jbrockmendel general comment: can you try to use a bit more descriptive PR titles? "Tslibs offsets paramd" does not say much to me :-)
> can you try to use a bit more descriptive PR titles

You got it.
> I mean update to a more proper doc-string

Sure thing.
pls hold off on this pending double-checking cross-DST behavior.
OK, I've convinced myself the DST issue is handled.
thanks, as always more tests are welcome! (w.r.t. DST)
> thanks, as always more tests are welcome! (w.r.t. DST)

I'm psyched to have this merged.  There's some follow-ups headed you're way I think you'll like.

w/r/t DST `shift_month` (and `shift_months`) is more fragile than I'd like, are safe largely because all existing usages are wrapped with `apply_wraps`.  But I don't want to rely on that always being the case.  So expect tests+fixes.

Specifically w/r/t to DST tests I've been thinking of trying hypothesis (#17978)