Skip to content
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

CLN: a batch of replacements of @Appender() with @doc() #33021

Conversation

smartvinnetou
Copy link
Contributor

@smartvinnetou smartvinnetou commented Mar 25, 2020

@smartvinnetou smartvinnetou force-pushed the CLN-31942/replace-appender-with-doc-2 branch 2 times, most recently from faec169 to 1b8420c Compare March 28, 2020 19:04
@smartvinnetou
Copy link
Contributor Author

I keep getting an error from a failed pytest. This happens in CI, see https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=31810&view=logs&j=3a03f79d-0b41-5610-1aa4-b4a014d0bc70&t=4d05ed0e-1ed3-5bff-dd63-1e957f2766a9
and locally
`(pandas-dev) C:\Users\David\source\repos\pandas-smartvinnetou>pytest pandas/tests/frame/methods/test_shift.py::TestDataFrameShift::test_shift_dt64values_int_fill_deprecated
=========================================================================================================================== test session starts ===========================================================================================================================
platform win32 -- Python 3.7.4, pytest-5.3.5, py-1.8.1, pluggy-0.13.1
rootdir: C:\Users\David\source\repos\pandas-smartvinnetou, inifile: setup.cfg
plugins: hypothesis-5.5.3, asyncio-0.10.0, cov-2.8.1, forked-1.1.3, xdist-1.31.0
collected 1 item

pandas\tests\frame\methods\test_shift.py F [100%]

================================================================================================================================ FAILURES =================================================================================================================================
______________________________________________________________________________________________________ TestDataFrameShift.test_shift_dt64values_int_fill_deprecated _______________________________________________________________________________________________________

self = <pandas.tests.frame.methods.test_shift.TestDataFrameShift object at 0x0000020E4201C088>

def test_shift_dt64values_int_fill_deprecated(self):
    # GH#31971
    ser = pd.Series([pd.Timestamp("2020-01-01"), pd.Timestamp("2020-01-02")])
    df = ser.to_frame()

    with tm.assert_produces_warning(FutureWarning):
      result = df.shift(1, fill_value=0)

pandas\tests\frame\methods\test_shift.py:195:


self = <contextlib._GeneratorContextManager object at 0x0000020E42021BC8>, 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: C:\Users\David\source\repos\pandas-smartvinnetou\pandas\core\frame.py != C:\Users\David\source\repos\pandas-smartvinnetou\pandas\tests\frame\methods\test_shift.py. W
arning message: Passing <class 'int'> to shift is deprecated and will raise in a future version, pass Timestamp instead.

......\AppData\Local\Programs\Python\Python37\lib\contextlib.py:119: AssertionError
============================================================================================================================ 1 failed in 0.50s ============================================================================================================================

(pandas-dev) C:\Users\David\source\repos\pandas-smartvinnetou>
`
I changed docstings and the related decorator. I have not changed any code. The error messages are not terribly informative.

Can someone shed some light on this failed test? Is this an issue with raising the wrong warning for some reason?

@smartvinnetou
Copy link
Contributor Author

After more investigation, I believe that the reason of the failed test the change of the depth of the stack introduced by the doc() decorator. It adds and extra frame per call to the stack (the wrapper() at https://github.com/pandas-dev/pandas/blob/master/pandas/util/_decorators.py#L224). The Appender() decorator changed the function it was decorating directly without introducing a wrapper, see https://github.com/pandas-dev/pandas/blob/master/pandas/util/_decorators.py#L381.

As the doc() decorator is called twice, there are two more frames on the stack (then there were when Appender() was used) when the FutureWarning() is raised at https://github.com/pandas-dev/pandas/blob/master/pandas/core/arrays/datetimelike.py#L790.

And then there is a mismatch when the failing test is checking the raised warning at https://github.com/pandas-dev/pandas/blob/master/pandas/_testing.py#L2366.

I can fix this mismatch by increasing the stacklevel at https://github.com/pandas-dev/pandas/blob/master/pandas/core/arrays/datetimelike.py#L795 to 9. This makes the actual_warning.filename and caller.filename values at https://github.com/pandas-dev/pandas/blob/master/pandas/_testing.py#L2372 point to pandas\\tests\\frame\\methods\\test_shift.py.

Can someone who knows the logic behind all this code confirm that this is the right change? @jorisvandenbossche ?

@smartvinnetou smartvinnetou marked this pull request as ready for review March 29, 2020 18:27
@smartvinnetou smartvinnetou force-pushed the CLN-31942/replace-appender-with-doc-2 branch from 98f0a5f to 314efe5 Compare March 30, 2020 20:31
@jorisvandenbossche
Copy link
Member

Can someone who knows the logic behind all this code confirm that this is the right change?

If the number of wrapping functions increased, then increasing the number specified in stacklevel is indeed the correct thing to do.

@jorisvandenbossche jorisvandenbossche changed the title a batch of replacements of @Appender() with @doc() CLN: a batch of replacements of @Appender() with @doc() Apr 1, 2020
@smartvinnetou smartvinnetou force-pushed the CLN-31942/replace-appender-with-doc-2 branch 2 times, most recently from 608bd82 to cbfb387 Compare April 4, 2020 08:23
@smartvinnetou smartvinnetou force-pushed the CLN-31942/replace-appender-with-doc-2 branch from cbfb387 to b23a01e Compare April 9, 2020 20:02
@jorisvandenbossche jorisvandenbossche merged commit acb525a into pandas-dev:master Apr 10, 2020
@jorisvandenbossche
Copy link
Member

@smartvinnetou thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants