DEPR: Deprecate cdate_range and merge into bdate_range #17691

jschendel · 2017-09-27T05:48:25Z

closes Merge cdate_range functionality into bdate_range #17596
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

A bit going on here, so see summary below.

Deprecated cdate_range:

Removed from api (was added to api after 0.20.3 release)
Added a FutureWarning to cdate_range indicating deprecation
- Added a test for this too
Converted other tests using cdate_range to use bdate_range instead
- Cleaned up the entire test script to make it more consistent with others (i.e. with context)
Removed a reference to cdate_range from the current whatsnew (previous commit I wrote)

Expanded functionality of bdate_range:

Added the additional cdate_range params (weekmask, holidays) for custom frequency ranges
Expanded that functionality beyond what was previously present in cdate_range
- cdate_range only supported cday with the weekmask and holidays params, now all custom freqs are supported.
- Did this by checking if the passed freq starts with 'C'.
- Added a test that should catch if a new non-custom freq is added that starts with 'C'.
- Didn't add this to cdate_range; left that as-is.
Added a UserWarning if non-default weekmask or holidays are passed without a custom freq.
- Added a test for this too.

Edits to timeseries.rst:

Added a custom freq bdate_range example to the "Generating Ranges of Timestamps" section
Reordered some of the examples in the "Generating Ranges of Timestamps" section
- Tried to order examples by simplicity
Otherwise, mostly small rewording changes and other such small fixes (ticks, capitalization, etc.).

codecov · 2017-09-27T06:42:25Z

Codecov Report

Merging #17691 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #17691      +/-   ##
==========================================
- Coverage   91.23%   91.21%   -0.02%     
==========================================
  Files         163      163              
  Lines       49796    49806      +10     
==========================================
+ Hits        45430    45431       +1     
- Misses       4366     4375       +9

Flag	Coverage Δ
#multiple	`89.01% <100%> (ø)`	⬆️
#single	`40.33% <30.76%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
pandas/tseries/offsets.py	`97.14% <ø> (-0.01%)`	⬇️
pandas/core/indexes/datetimes.py	`95.58% <100%> (+0.04%)`	⬆️
pandas/core/api.py	`100% <100%> (ø)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.73% <0%> (-0.1%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 408ecd2...4cbf70d. Read the comment docs.

jreback

looks pretty good

jreback · 2017-09-27T10:09:58Z

doc/source/timeseries.rst

 .. _timeseries.representation:

-Time Stamps vs. Time Spans
+Timestamps vs. Time Spans


underlines should match the length exactly

jreback · 2017-09-27T10:11:14Z

doc/source/timeseries.rst

+``DatetimeIndex``. The default unit is nanoseconds, since that is how ``Timestamp``
+objects are stored internally. However, epochs are often stored in another ``unit``
+which can be specified. These are computed from the starting point specified by the
+:ref:`Origin Parameter <timeseries.origin>`.


use origin

jreback · 2017-09-27T10:12:18Z

doc/source/timeseries.rst


   pd.bdate_range(start=start, periods=20)

-The start and end dates are strictly inclusive. So it will not generate any


put a small ::warning box where you say cdate_range is deprecated, add a ref as well (then you can refer to it in whatsnew)

in the appropriate location in this file

jreback · 2017-09-27T10:14:05Z

doc/source/whatsnew/v0.21.0.txt

 - :func:`MultiIndex.is_monotonic_decreasing` has been implemented.  Previously returned ``False`` in all cases. (:issue:`16554`)
 - :func:`Categorical.rename_categories` now accepts a dict-like argument as `new_categories` and only updates the categories found in that dict. (:issue:`17336`)
 - :func:`read_excel` raises ``ImportError`` with a better message if ``xlrd`` is not installed. (:issue:`17613`)
 - :meth:`DataFrame.assign` will preserve the original order of ``**kwargs`` for Python 3.6+ users instead of sorting the column names


move to deprecation section, mention the deprecation, and include a ref

jreback · 2017-09-27T10:14:24Z

doc/source/whatsnew/v0.21.0.txt

 - ``pd.options.html.border`` has been deprecated in favor of ``pd.options.display.html.border`` (:issue:`15793`).
 - :func:`SeriesGroupBy.nth` has deprecated ``True`` in favor of ``'all'`` for its kwarg ``dropna`` (:issue:`11038`).
 - :func:`DataFrame.as_blocks` is deprecated, as this is exposing the internal implementation (:issue:`17302`)
+- ``cdate_range`` has been deprecated in favor of :func:`bdate_range` (:issue:`17596`)


combine with the previous

jreback · 2017-09-27T10:15:48Z

pandas/core/indexes/datetimes.py

+            msg = 'invalid custom frequency string: {freq}'.format(freq=freq)
+            raise ValueError(msg)
+    elif holidays or (weekmask != 'Mon Tue Wed Thu Fri'):
+        warnings.warn('a custom frequency string was not passed, ignoring '


raise here, this is invalid

I agree this can raise.

jreback · 2017-09-27T10:17:00Z

pandas/tests/indexes/datetimes/test_date_range.py

        tm.assert_index_equal(result_2, expected_2)


 class TestCustomDateRange(object):


separately, most of these classes could actually be a hierarchy instead to share some code (or could parametrize things), but separate / later PR.

jreback · 2017-09-27T10:18:25Z

pandas/tests/indexes/datetimes/test_date_range.py

+        with tm.assert_raises_regex(ValueError, msg.format(freq=bad_freq)):
+            bdate_range(START, END, freq=bad_freq)
+
+    def test_depr_cdaterange(self):


deprecation

jorisvandenbossche

Thanks a lot for this!
Added a few comments

jorisvandenbossche · 2017-09-28T08:01:50Z

doc/source/timeseries.rst


 .. ipython:: python

+   pd.Timestamp(1349720105, unit='s')


I personally don't find it necessary to add an example using Timestamp, IMO we should 'sell' to_datetime as the function to convert things to timestamps. (you can also convert a scalar with to_datetime if you want)

jorisvandenbossche · 2017-09-28T08:03:18Z

doc/source/timeseries.rst

-frequency, we can use the pandas functions ``date_range`` and ``bdate_range``
-to create timestamp indexes.
+frequency, we can use the ``date_range`` and ``bdate_range`` functions
+to create a ``DatetimeIndex``. The default frequency for ``date_range`` is a


can you make date_range and bdate_range references to their docstring? (:func:`date_range` )

jorisvandenbossche · 2017-09-28T08:06:11Z

doc/source/timeseries.rst

-dates outside of those dates if specified.
+.. versionadded:: 0.21.0
+
+``bdate_range`` can also generate a range of custom frequency dates by using


I think if you indent this paragraph, it more 'belongs' to the versionadded directive (although not fully sure how it will look like when rendered)

I tried indenting and it rendered everything on the same line, i.e. "New in version 0.21.0: bdate_range can also...". I changed it back to make it consistent with the other usages of versionadded within timeseries.rst and other documentation.

jorisvandenbossche · 2017-09-28T08:11:30Z

pandas/core/indexes/datetimes.py


 def bdate_range(start=None, end=None, periods=None, freq='B', tz=None,
-                normalize=True, name=None, closed=None, **kwargs):
+                normalize=True, name=None, weekmask='Mon Tue Wed Thu Fri',


I would put the weekmask default to None? (as it is only used when freq is something else, so this gives a bit a misleading signature?

And if freq='C' is passed and weekmask is None, you can set it to this value.
(or is None a valid value when freq='C' ?)

jorisvandenbossche · 2017-09-28T08:12:50Z

pandas/core/indexes/datetimes.py

    name : string, default None
        Name of the resulting DatetimeIndex
+    weekmask : string, default 'Mon Tue Wed Thu Fri'
+        weekmask of valid business days, passed to ``numpy.busdaycalendar``,


weekmask -> Weekmask (also the description for 'holidays' can start with capital)

jorisvandenbossche · 2017-09-28T08:14:40Z

pandas/core/indexes/datetimes.py

+            msg = 'invalid custom frequency string: {freq}'.format(freq=freq)
+            raise ValueError(msg)
+    elif holidays or (weekmask != 'Mon Tue Wed Thu Fri'):
+        warnings.warn('a custom frequency string was not passed, ignoring '


I agree this can raise.

jorisvandenbossche · 2017-09-28T08:16:27Z

pandas/core/indexes/datetimes.py

    rng : DatetimeIndex
    """
+    warnings.warn("cdate_range is deprecated and will be removed in a future "
+                  "version, instead use bdate_range(..., freq='{freq}')"


bdate_range -> pd.bdate_range (to make it clear that bdate_range is top-level, as opposed to cdate_range)

jschendel · 2017-09-29T04:22:10Z

Made the review changes and fixed some unrelated typos I noticed in whatsnew (missing : and `).

jorisvandenbossche

Looks good!

jorisvandenbossche · 2017-09-29T07:55:04Z

doc/source/timeseries.rst

   pd.Timestamp.min
   pd.Timestamp.max

-See :ref:`here <timeseries.oob>` for ways to represent data outside these bound.


I think here the "for ways to represent data outside these bound" is actually useful to know that it is interesting to click this link

Ah, but the 'see also' block will show the full title, which is "representing out of bounds spans", which also says it.

jreback · 2017-10-01T16:37:12Z

doc/source/timeseries.rst

+
+   pd.bdate_range(start, end, freq='CBMS', weekmask=weekmask)
+
+.. warning::


move this to the top of the subsection. That way you can in the whatsnew refer directly to this section.

jreback · 2017-10-01T16:39:26Z

doc/source/whatsnew/v0.21.0.txt

 Categorical
 ^^^^^^^^^^^
- Bug in :func:`Series.isin` when called with a categorical (:issue`16639`)
+- Bug in :func:`Series.isin` when called with a categorical (:issue:`16639`)


thanks for fixing this!

jreback · 2017-10-01T16:41:03Z

pandas/tests/api/test_api.py

    funcs = ['bdate_range', 'concat', 'crosstab', 'cut',
             'date_range', 'interval_range', 'eval',
-             'factorize', 'get_dummies', 'cdate_range',
+             'factorize', 'get_dummies',


move to the deprecated functions section, and add a test (in same module), model after the top-level deprecations.

jreback · 2017-10-01T16:42:30Z

pandas/tseries/offsets.py

    FY5253,
    FY5253Quarter,
 ])
-


why did you change this?

Cleaning. Seemed a bit strange that Nano was being added to the dict after creation. Looks like it was historically structured that way for compat with numpy < 1.7:

pandas/pandas/tseries/offsets.py

Lines 2017 to 2019 in 94e2194

if not _np_version_under1p7:

# Only 1.7+ supports nanosecond resolution

prefix_mapping['N'] = Nano

Structure was kept the same when the if statement was removed, but seems fine to directly add it to the dict during creation now.

jschendel · 2017-10-02T04:47:03Z

Made the additional review changes.

jreback · 2017-10-02T10:11:04Z

pandas/tests/api/test_api.py

+
+    def test_deprecation_cdaterange(self):
+        # GH17596
+        from pandas.core.indexes.datetimes import cdate_range


import from pandas here (we are actually testing the pd. deprecation); in this case they are the same, but its our practice to use the one we are deprecating.

no, we are testing this one (the move to top-level pd namespace was only very recently in master, so we don't have to deprecate that. That one we just removed, and we are deprecating the actual nested one)

oh, ok then; guess we moved this in 0.21.0, so this is fine then.

jreback

just a minor comment.

jreback · 2017-10-02T10:12:11Z

lgtm. @jorisvandenbossche @TomAugspurger if you have anything further.

jreback · 2017-10-02T11:28:34Z

thanks @jschendel

* 'master' of github.com:pandas-dev/pandas: (188 commits) Separate out _convert_datetime_to_tsobject (pandas-dev#17715) DOC: remove whatsnew note for xref pandas-dev#17131 BUG: Regression in .loc accepting a boolean Index as an indexer (pandas-dev#17738) DEPR: Deprecate cdate_range and merge into bdate_range (pandas-dev#17691) CLN: replace %s syntax with .format in pandas.core: categorical, common, config, config_init (pandas-dev#17735) Fixed the memory usage explanation of categorical in gotchas from O(nm) to O(n+m) (pandas-dev#17736) TST: add backward compat for offset testing for pickles (pandas-dev#17733) remove unused time conversion funcs (pandas-dev#17711) DEPR: Deprecate convert parameter in take (pandas-dev#17352) BUG:Time Grouper bug fix when applied for list groupers (pandas-dev#17587) BUG: Fix some PeriodIndex resampling issues (pandas-dev#16153) BUG: Fix unexpected sort in groupby (pandas-dev#17621) DOC: Fixed typo in documentation for 'pandas.DataFrame.replace' (pandas-dev#17731) BUG: Fix series rename called with str altering name rather index (GH17407) (pandas-dev#17654) DOC: Add examples for MultiIndex.get_locs + cleanups (pandas-dev#17675) Doc improvements for IntervalIndex and Interval (pandas-dev#17714) BUG: DataFrame sort_values and multiple "by" columns fails to order NaT correctly Last of the timezones funcs (pandas-dev#17669) Add missing file to _pyxfiles, delete commented-out (pandas-dev#17712) update imports of DateParseError, remove unused imports from tslib (pandas-dev#17713) ...

)

jreback requested changes Sep 27, 2017

View reviewed changes

jreback added Deprecate Functionality to remove in pandas Frequency DateOffsets labels Sep 27, 2017

jsexauer mentioned this pull request Sep 27, 2017

DEPR: Clean up list of deprecations from prior versions #6581

Closed

1 task

jorisvandenbossche reviewed Sep 28, 2017

View reviewed changes

jorisvandenbossche added this to the 0.21.0 milestone Sep 28, 2017

jschendel force-pushed the depr-cdate_range branch from eb38d12 to 7306a73 Compare September 29, 2017 04:19

jorisvandenbossche approved these changes Sep 29, 2017

View reviewed changes

jreback requested changes Oct 1, 2017

View reviewed changes

jschendel added 3 commits October 1, 2017 17:51

DEPR: Deprecate cdate_range merge into bdate_range

5393748

review updates

3d58a51

review updates 2

4cbf70d

jschendel force-pushed the depr-cdate_range branch from 7306a73 to 4cbf70d Compare October 2, 2017 03:16

jreback reviewed Oct 2, 2017

View reviewed changes

jreback approved these changes Oct 2, 2017

View reviewed changes

TomAugspurger approved these changes Oct 2, 2017

View reviewed changes

jreback merged commit 2781b18 into pandas-dev:master Oct 2, 2017

jschendel deleted the depr-cdate_range branch October 2, 2017 13:39

alanbato pushed a commit to alanbato/pandas that referenced this pull request Nov 10, 2017

DEPR: Deprecate cdate_range and merge into bdate_range (pandas-dev#17691

7d9e789

)

No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017

DEPR: Deprecate cdate_range and merge into bdate_range (pandas-dev#17691

c280a6b

)

jreback mentioned this pull request Jun 30, 2019

DEPR: deprecations log for removed issues #13777

Closed


		pd.bdate_range(start=start, periods=20)

		The start and end dates are strictly inclusive. So it will not generate any

		tm.assert_index_equal(result_2, expected_2)


		class TestCustomDateRange(object):


		pd.bdate_range(start, end, freq='CBMS', weekmask=weekmask)

		.. warning::

	if not _np_version_under1p7:
	# Only 1.7+ supports nanosecond resolution
	prefix_mapping['N'] = Nano

Uh oh!

DEPR: Deprecate cdate_range and merge into bdate_range #17691

DEPR: Deprecate cdate_range and merge into bdate_range #17691

Uh oh!

Conversation

jschendel commented Sep 27, 2017

Uh oh!

codecov bot commented Sep 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jreback left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jschendel commented Sep 29, 2017

Uh oh!

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jschendel Oct 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jschendel commented Oct 2, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 27, 2017 •

edited

Loading

jschendel Oct 1, 2017 •

edited

Loading

jreback Oct 2, 2017 •

edited

Loading