DEPR: Add Deprecated warning for timedelta with passed units M and Y #23264

ryankarlos · 2018-10-21T17:36:32Z

closes DEPR: Timedelta with passed unit M, Y #16344
passes git diff upstream/master -u -- "*.py" | flake8 --diff
tests passed locally - pytest pandas/tests/scalar/timedelta/test_timedelta.py -k test_unit_m_y_deprecated and pytest pandas/tests/indexes/timedeltas/test_timedelta.py -k test_unit_m_y_deprecated
checked import format to pass linter - isort

pep8speaks · 2018-10-21T17:36:35Z

Hello @ryankarlos! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on February 08, 2019 at 22:56 Hours UTC

ryankarlos · 2018-10-21T17:39:10Z

Just made a related PR #23259 for timedelta docstring so thought I would give this a try as well. This is an initial PR to check if Im on the right track - my first time adding deprecated warnings.

pandas/core/tools/timedeltas.py

mroeschke · 2018-10-21T22:38:15Z

Could you add some tests that validate that the warning is raised? Would need to check Timedelta, TimedeltaIndex, and to_timedelta.

Also there might be some existing tests that use those offsets, so could you check the logs and see if any tests are raising the warning as well?

ryankarlos · 2018-10-24T19:52:28Z

Sure will work on it.

jreback

need to have a specific test for this
where we are testing this now, need to catch the warnings (or can just call that part 1 is ok)
whatsnew note in the deprecation section

pandas/core/tools/timedeltas.py

sinhrks · 2018-11-01T03:09:26Z

Deprecating Timedelta may be easier after #23439

ryankarlos · 2018-11-12T21:13:47Z

@mroeschke Ive added an update with tests for deprecating units - please have a look. Ive fetched the latest merge #23439 to include the changes from there as well.

pandas/core/tools/timedeltas.py

pandas/tests/scalar/timedelta/test_timedelta.py

mroeschke · 2018-11-13T00:57:27Z

Be sure to also check the CI runs (travis) to see where else 'M' and 'Y' might be used with timedelta. We will need to catch the deprecation behavior of other tests that use this behavior.

codecov · 2018-11-23T00:46:12Z

Codecov Report

Merging #23264 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #23264      +/-   ##
==========================================
- Coverage   92.15%   92.15%   -0.01%     
==========================================
  Files         166      166              
  Lines       52294    52299       +5     
==========================================
+ Hits        48194    48198       +4     
- Misses       4100     4101       +1

Flag	Coverage Δ
#multiple	`90.62% <100%> (ø)`	⬆️
#single	`42.27% <60%> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/tools/timedeltas.py	`98.11% <100%> (+0.11%)`	⬆️
pandas/core/indexes/timedeltas.py	`90.27% <100%> (+0.05%)`	⬆️
pandas/util/testing.py	`87.89% <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 1d1b14c...76497d3. Read the comment docs.

ryankarlos · 2018-11-23T01:41:28Z

@mroeschke sorry for the delay - ive added those fixes in now. I could see around 6 failures in Travis - showing deprecation warnings for test_unit_parser using M and Y.

pandas/tests/scalar/timedelta/test_timedelta.py

pandas/core/tools/timedeltas.py

jreback · 2018-12-14T15:51:26Z

can you merge master and upate

ryankarlos · 2018-12-16T03:05:05Z

@jreback Yes sure, sorry somehow I had missed your replies to my previous commits.

ryankarlos · 2018-12-16T18:57:47Z

@mroeschke I think I may need to add a warning for Timedelta as its not producing any FutureWarning making the tests fail - im not sure where this would need to be added though

mroeschke · 2018-12-27T18:28:50Z

Sorry for the delay @ryankarlos.

You can add the FutureWarning here for the Timedelta scalar:

pandas/pandas/_libs/tslibs/timedeltas.pyx

Line 1143 in 7e64d9c

def __new__(cls, object value=_no_input, unit=None, **kwargs):

pandas/tests/scalar/timedelta/test_timedelta.py

doc/source/whatsnew/v0.24.0.rst

jorisvandenbossche

Some small comments, but looking good!

pandas/tests/indexes/timedeltas/test_timedelta.py

pandas/tests/scalar/timedelta/test_timedelta.py

jorisvandenbossche · 2019-02-03T16:32:53Z

pandas/tests/scalar/timedelta/test_timedelta.py

+    @pytest.mark.parametrize('unit', ['Y', 'y', 'M'])
+    def test_unit_m_y_deprecated(self, unit):
+        with tm.assert_produces_warning(FutureWarning,
+                                        check_stacklevel=False) as w1:


Can you remove the check_stacklevel=False here?
Here it should be work, in the one below for to_timedelta, it is fine to keep.

@jorisvandenbossche I just tried running this locally and i get an assertion error if I remove check_stacklevel for Timdelta (setting stacklevel = 2 or stacklevel=3 seems to make no difference). Not sure if it will get through travis if i don't set to check_stacklevel=False- i remember experimenting with this in the previous builds which failed.:

../../../anaconda/envs/pandas-dev/lib/python3.7/contextlib.py:119: AssertionError
________________________________ TestTimedeltas.test_unit_m_y_deprecated[M] _________________________________

self = <pandas.tests.scalar.timedelta.test_timedelta.TestTimedeltas object at 0x1c169e02b0>, unit = 'M'

@pytest.mark.skipif(sys.version_info < (3, 5), reason="requires python3.5 or higher") @pytest.mark.parametrize('unit', ['Y', 'y', 'M']) def test_unit_m_y_deprecated(self, unit): with tm.assert_produces_warning(FutureWarning) as w1:

Timedelta(10, unit)

pandas/tests/scalar/timedelta/test_timedelta.py:387:

self = <contextlib._GeneratorContextManager object at 0x1c169e0438>, type = None, value = None
traceback = None

def __exit__(self, type, value, traceback): if type is None: try:

next(self.gen)

E AssertionError: Warning not set with correct stacklevel. File where warning is raised: /Users/ryannazareth/anaconda/envs/pandas-dev/lib/python3.7/site-packages/_pytest/python.py != /Users/ryannazareth/Documents/Python_sprints/pandas-ryankarlos/pandas/tests/scalar/timedelta/test_timedelta.py. Warning message: M and Y units are deprecated and will be removed in a future version.

../../../anaconda/envs/pandas-dev/lib/python3.7/contextlib.py:119: AssertionError
================================== 3 failed, 75 deselected in 0.68 seconds ==================================

pandas/_libs/tslibs/timedeltas.pyx

ryankarlos · 2019-02-03T17:59:11Z

@jorisvandenbossche i've pushed latest changes with removing checkstacklevel for timedelta test as requested-- if it fails is it ok to revert ?

…medelta_unit_depracated

…iling

jorisvandenbossche · 2019-02-03T20:51:50Z

@ryankarlos tested it locally, and apparently it needs to be 1 for Timedelta, and not 2.

ryankarlos · 2019-02-03T22:08:17Z

I just tried with 1 for Timedelta locally but still getting the assertion error

jorisvandenbossche · 2019-02-03T22:13:32Z

Did you rebuild pandas before testing? The change is in a cython .pyx file, so you need to rebuild the C extensions before the change will be in effect.

…medelta_unit_depracated

ryankarlos · 2019-02-04T00:28:41Z

thanks, Ok it passes now but I had to merge master and build C extensions on a fresh pandas build - wasn't working when cythonizing on the previous env for some reason.

ryankarlos · 2019-02-04T00:59:31Z

@jreback @jorisvandenbossche all green now

jorisvandenbossche

Thanks for the update! Two more minor comments

pandas/_libs/tslibs/timedeltas.pyx

pandas/tests/scalar/timedelta/test_timedelta.py

jreback

small comments and some from @jorisvandenbossche ping when updated and on green

doc/source/whatsnew/v0.24.0.rst

pandas/tests/scalar/timedelta/test_timedelta.py

…medelta_unit_depracated

ryankarlos · 2019-02-08T23:31:51Z

@jreback All green now...... I merged master and added the requested changes.

jreback · 2019-02-08T23:37:48Z

thanks @ryankarlos nice PR!

ryankarlos · 2019-02-08T23:42:59Z

@jreback thanks, will hopefully get a few more in before the 0.25 release !

mroeschke reviewed Oct 21, 2018

View reviewed changes

pandas/core/tools/timedeltas.py Outdated Show resolved Hide resolved

mroeschke added Timedelta Timedelta data type Deprecate Functionality to remove in pandas labels Oct 21, 2018

ryankarlos mentioned this pull request Oct 25, 2018

DOC: Fixed timedelta docstring and examples #23259

Merged

4 tasks

jreback requested changes Oct 26, 2018

View reviewed changes

pandas/core/tools/timedeltas.py Outdated Show resolved Hide resolved

sinhrks mentioned this pull request Nov 1, 2018

BUG: Cleanup timedelta offset #23439

Merged

4 tasks

ryankarlos force-pushed the timedelta_unit_depracated branch from 4963cb1 to 09eb209 Compare November 12, 2018 21:09

mroeschke reviewed Nov 13, 2018

View reviewed changes

pandas/core/tools/timedeltas.py Outdated Show resolved Hide resolved

mroeschke reviewed Nov 13, 2018

View reviewed changes

pandas/core/tools/timedeltas.py Outdated Show resolved Hide resolved

mroeschke reviewed Nov 13, 2018

View reviewed changes

pandas/tests/scalar/timedelta/test_timedelta.py Outdated Show resolved Hide resolved

ryankarlos force-pushed the timedelta_unit_depracated branch from 09eb209 to ad28eff Compare November 23, 2018 00:46

jreback requested changes Nov 23, 2018

View reviewed changes

pandas/tests/scalar/timedelta/test_timedelta.py Outdated Show resolved Hide resolved

pandas/core/tools/timedeltas.py Outdated Show resolved Hide resolved

ryankarlos force-pushed the timedelta_unit_depracated branch 2 times, most recently from 1f779ef to 74a48bd Compare December 16, 2018 17:58

mroeschke reviewed Dec 27, 2018

View reviewed changes

pandas/tests/scalar/timedelta/test_timedelta.py Outdated Show resolved Hide resolved

mroeschke reviewed Dec 27, 2018

View reviewed changes

pandas/tests/scalar/timedelta/test_timedelta.py Outdated Show resolved Hide resolved

mroeschke reviewed Dec 27, 2018

View reviewed changes

pandas/tests/scalar/timedelta/test_timedelta.py Outdated Show resolved Hide resolved

alimcmaster1 reviewed Feb 3, 2019

View reviewed changes

doc/source/whatsnew/v0.24.0.rst Outdated Show resolved Hide resolved

jorisvandenbossche reviewed Feb 3, 2019

View reviewed changes

Minor changes

3f87a0b

ryankarlos added 2 commits February 3, 2019 18:30

Merge branch 'master' of https://github.com/pandas-dev/pandas into ti…

b7aedc1

…medelta_unit_depracated

Reverting back to checkstacklevel=False for Timdelta as Travis was fa…

f44bae0

…iling

ryankarlos added 2 commits February 3, 2019 22:34

Merge branch 'master' of https://github.com/pandas-dev/pandas into ti…

5d2dceb

…medelta_unit_depracated

set stacklevel=1 for Timedelta and remove checkstacklevel=False

b745b8c

jorisvandenbossche reviewed Feb 4, 2019

View reviewed changes

pandas/_libs/tslibs/timedeltas.pyx Outdated Show resolved Hide resolved

pandas/tests/scalar/timedelta/test_timedelta.py Show resolved Hide resolved

jreback requested changes Feb 6, 2019

View reviewed changes

doc/source/whatsnew/v0.24.0.rst Outdated Show resolved Hide resolved

pandas/tests/scalar/timedelta/test_timedelta.py Outdated Show resolved Hide resolved

ryankarlos added 3 commits February 8, 2019 20:29

Merge branch 'master' of https://github.com/pandas-dev/pandas into ti…

d014f56

…medelta_unit_depracated

added requested changes and removed unused sys import - linter

ca0ea09

reverting file - removed added line

76497d3

jreback approved these changes Feb 8, 2019

View reviewed changes

jreback merged commit 04df22f into pandas-dev:master Feb 8, 2019

mroeschke mentioned this pull request Mar 5, 2019

Pandas TimeDelta instantiation returns different object for same value #25544

Closed

jbrockmendel mentioned this pull request Dec 6, 2019

DEPR: YyM units for Timedelta and friends #30118

Merged

jreback mentioned this pull request Dec 7, 2019

DEPR: deprecations log for removed issues #13777

Closed

DaveCasson mentioned this pull request Aug 3, 2023

Incompatibility with newest Pandas 2.0.0 UW-Hydro/MetSim#258

Closed

Uh oh!

DEPR: Add Deprecated warning for timedelta with passed units M and Y #23264

DEPR: Add Deprecated warning for timedelta with passed units M and Y #23264

Conversation

ryankarlos commented Oct 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pep8speaks commented Oct 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated on February 08, 2019 at 22:56 Hours UTC

Uh oh!

ryankarlos commented Oct 21, 2018

Uh oh!

Uh oh!

mroeschke commented Oct 21, 2018

Uh oh!

ryankarlos commented Oct 24, 2018

Uh oh!

jreback left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sinhrks commented Nov 1, 2018

Uh oh!

ryankarlos commented Nov 12, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mroeschke commented Nov 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ryankarlos commented Nov 23, 2018

Uh oh!

Uh oh!

Uh oh!

jreback commented Dec 14, 2018

Uh oh!

ryankarlos commented Dec 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryankarlos commented Dec 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mroeschke commented Dec 27, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jorisvandenbossche Feb 3, 2019

Choose a reason for hiding this comment

Uh oh!

ryankarlos Feb 3, 2019 • 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!

Uh oh!

ryankarlos commented Feb 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jorisvandenbossche commented Feb 3, 2019

Uh oh!

ryankarlos commented Feb 3, 2019

Uh oh!

jorisvandenbossche commented Feb 3, 2019

Uh oh!

ryankarlos commented Feb 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryankarlos commented Feb 4, 2019

ryankarlos commented Oct 21, 2018 •

edited

Loading

pep8speaks commented Oct 21, 2018 •

edited

Loading

mroeschke commented Nov 13, 2018 •

edited

Loading

codecov bot commented Nov 23, 2018 •

edited

Loading

ryankarlos commented Dec 16, 2018 •

edited

Loading

ryankarlos commented Dec 16, 2018 •

edited

Loading

ryankarlos Feb 3, 2019 •

edited

Loading

ryankarlos commented Feb 3, 2019 •

edited

Loading

ryankarlos commented Feb 4, 2019 •

edited

Loading