CLN: Remove deprecated method #22324
Conversation
| weekmask of valid business days, passed to ``numpy.busdaycalendar`` | ||
| holidays : list | ||
| list/array of dates to exclude from the set of valid business days, | ||
| passed to ``numpy.busdaycalendar`` |
There was a problem hiding this comment.
If you look at the implementation, although they may not be explicit parameters, but they should be documented. Not sure if plain removal is the right move here.
There was a problem hiding this comment.
Ahh apologies I looked at bdate_range and saw they where optional params there opposed to kwargs. It's a deprecated method anyway, but how about documenting it something like below So its clear they are kwargs?
closed : string, default None
Make the interval closed with respect to the given frequency to
the 'left', 'right', or both sides (None)
**weekmask : string, Default 'Mon Tue Wed Thu Fri'
weekmask of valid business days, passed to ``numpy.busdaycalendar``
**holidays : list
list/array of dates to exclude from the set of valid business days,
passed to ``numpy.busdaycalendar``
There was a problem hiding this comment.
Actually, I'll do you one better. Let's just remove it (it's been long enough).
How does that sound?
doc/source/timeseries.rst
Outdated
| ``bdate_range``. Note that ``cdate_range`` only utilizes the ``weekmask`` | ||
| and ``holidays`` parameters when custom business day, 'C', is passed | ||
| as the frequency string. Support has been expanded with ``bdate_range`` | ||
| to work with any custom frequency string. |
There was a problem hiding this comment.
Do we still need to reference cdate_range in the docs? Or can we rewrite to remove it?
There was a problem hiding this comment.
Im thinking of just removing that warning altogether since it just mentions some subtleties of cdate_range. Alternatively potential rewrite could look like:
This functionality was originally exclusive to ``cdate_range``, which is deprecated as of version 0.21.0 (deleted in 0.24.0) in favor of ``bdate_range`` which works with any custom frequency string.
Thoughts?
There was a problem hiding this comment.
I would just remove it altogether.
There was a problem hiding this comment.
Makes sense! updated
Codecov Report
@@ Coverage Diff @@
## master #22324 +/- ##
==========================================
- Coverage 92.08% 92.08% -0.01%
==========================================
Files 169 169
Lines 50706 50699 -7
==========================================
- Hits 46691 46684 -7
Misses 4015 4015
Continue to review full report at Codecov.
|
| ``cdate_range`` only utilizes the ``weekmask`` and ``holidays`` parameters | ||
| when custom business day, 'C', is passed as the frequency string. Support has | ||
| been expanded with ``bdate_range`` to work with any custom frequency string. | ||
|
|
||
| .. versionadded:: 0.21.0 |
There was a problem hiding this comment.
I think this line can be removed as well.
|
we are not removing 0.21.0 deprecations yet, see the list here: #6581 things deprecated in 0.20.0 are fair game however. |
|
Okay no worries, thanks @jreback for the info! |
|
@jreback : That was my mistake. I was under the impression that @alimcmaster1 : Sorry about that! I think given we'll remove this method soon, let's leave the docs as is. We can always make this removal later. |
These params are no longer in the method, therefore can remove the docs strings