-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DEPR update dedeprecation message for np.ptp #28665 #6581 #30401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DEPR update dedeprecation message for np.ptp #28665 #6581 #30401
Conversation
The fix here is not to update the warning but to remove Series.ptp entirely |
Here in #28665 |
pandas/core/generic.py
Outdated
@@ -10198,7 +10198,10 @@ def nanptp(values, axis=0, skipna=True): | |||
nmin = nanops.nanmin(values, axis, skipna) | |||
warnings.warn( | |||
"Method .ptp is deprecated and will be removed " | |||
"in a future version. Use numpy.ptp instead.", | |||
"in a future version. Use numpy.ptp instead." | |||
"if you are already using numpy.ptp and still getting this message" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"if you are already using numpy.ptp and still getting this message" | |
"if you are already using numpy.ptp and still getting this message, " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
pandas/core/generic.py
Outdated
"in a future version. Use numpy.ptp instead.", | ||
"in a future version. Use numpy.ptp instead." | ||
"if you are already using numpy.ptp and still getting this message" | ||
"please call to_numpy() to avoid this message in future calls" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"please call to_numpy() to avoid this message in future calls" | |
"please call to_numpy() to avoid this message in future calls. " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
pandas/core/generic.py
Outdated
"in a future version. Use numpy.ptp instead." | ||
"if you are already using numpy.ptp and still getting this message" | ||
"please call to_numpy() to avoid this message in future calls" | ||
"eg : np.ptp(pd.Series([1, 2, 3]).to_numpy())", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"eg : np.ptp(pd.Series([1, 2, 3]).to_numpy())", | |
"For example: np.ptp(pd.Series([1, 2, 3]).to_numpy())", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree with @jbrockmendel let's just remove this entirely. it has been deprecated for a while and we are removing code for 1.0
…dev#6581 requested changes made
so, for now, I have updated the message, |
no, for 1.0 we are going to remove this code, if you want to repurpose this PR to do it great. |
Ohhk no isssues |
ok, you remove the code & associated tests, see https://dev.pandas.io/docs/whatsnew/v1.0.0.html#removal-of-prior-version-deprecations-changes for things that we have removed, so add an entry liek this |
so I removed code and tests for ptp ` self = <pandas.tests.series.test_analytics.TestSeriesAnalytics object at 0x7f666e0267b8>
pandas/tests/series/test_analytics.py:861: self = <contextlib._GeneratorContextManager object at 0x7f666e03ad30>, type = None, value = None, traceback = None
E AssertionError: Did not see expected warning of class 'FutureWarning' /usr/lib/python3.6/contextlib.py:88: AssertionError |
pandas/core/generic.py
Outdated
``numpy.ndarray`` method ``ptp``.""", | ||
nanptp, | ||
) | ||
# @classmethod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete all of this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
pandas/core/series.py
Outdated
@@ -4390,7 +4390,7 @@ def to_period(self, freq=None, copy=True): | |||
["index"], docs={"index": "The index (axis labels) of the Series."}, | |||
) | |||
Series._add_numeric_operations() | |||
Series._add_series_only_operations() | |||
# Series._add_series_only_operations() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete. basically never leave commented-out code in a PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually made this commit to get some help on getting this future warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so I removed code and tests for ptp
remove the part where series only functions are added
removed the entire series only function because ptp was the only series only function
deleted tests for pandas ptp function
but the test written for np.ptp gives me a future warning`
____________________________________________________________ TestSeriesAnalytics.test_ptp ____________________________________________________________self = <pandas.tests.series.test_analytics.TestSeriesAnalytics object at 0x7f666e0267b8>
def test_ptp(self): # GH21614 N = 1000 arr = np.random.randn(N) ser = Series(arr) with tm.assert_produces_warning(FutureWarning, check_stacklevel=False):
assert np.ptp(ser) == np.ptp(arr)
pandas/tests/series/test_analytics.py:861:
self = <contextlib._GeneratorContextManager object at 0x7f666e03ad30>, type = None, value = None, traceback = None
def __exit__(self, type, value, traceback): if type is None: try:
next(self.gen)
E AssertionError: Did not see expected warning of class 'FutureWarning'
/usr/lib/python3.6/contextlib.py:88: AssertionError
================================================ 1 failed, 163 passed, 1 skipped, 3 xfailed in 1.96s =================================================
`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the assert_produces_warning, since it shouldn't anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it works but can someone please explain to me the reason behind it so that I learn
tests are not failing at local no clue why azure pipelines are failing ` pandas/tests/series/test_analytics.py ..........................s....... [ 20%] ================== 164 passed, 1 skipped, 3 xfailed in 3.26s =================== |
What was happening in the failing builds is that when we call |
So I actually had tried what is numpy returning on my local machine. Over there it's returning float |
…e and values of ptp to understand why azure build failing
As I am unable to reproduce this on my local machine I tried to push a commit with the print statements which are ignored by the pipeline. mac os python 3.6 checking the logs show me that the series returned by numpy is all True boolean series 2019-12-25T01:57:55.8017700Z self = 0 True if it would have been returning a series with just the correct values it would have been simple to extract it, but the returned series is not correct. |
At last all tests passing |
thanks for the effort @hasnain2808 I had already merged #30458 happy to take a contribution on other issues. |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff