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

DEPR: add stacklevel to FutureWarnings (GH9584) #10676

Merged
merged 3 commits into from
Sep 4, 2015

Conversation

jorisvandenbossche
Copy link
Member

Closes #9584

@jorisvandenbossche jorisvandenbossche added Testing pandas testing functions or related to the test suite Deprecate Functionality to remove in pandas labels Jul 26, 2015
@jorisvandenbossche jorisvandenbossche added this to the 0.17.0 milestone Jul 26, 2015
@jreback
Copy link
Contributor

jreback commented Aug 11, 2015

should revamp this....its pretty useful :)

@jorisvandenbossche
Copy link
Member Author

yes, on my todo list!
The problem I encountered is that for certain Warnings that are raised deeper into the pandas internals, the needed stacklevel is not always the same for different end user use cases. So it will not be possible to fix all of them. But for many (all warnings of deprecated functions or keywords, which are raised rather top-level), it is also simple to fix.

I now patched the assert_produced_warning to check for this, but should make it option so you can turn if off when it is not possible to get it right.

@jreback
Copy link
Contributor

jreback commented Aug 11, 2015

@jorisvandenbossche sounds good. Well at least the top-level ones would be a step up.

@jorisvandenbossche jorisvandenbossche force-pushed the stacklevel branch 4 times, most recently from d83058d to 96301d1 Compare August 16, 2015 14:28
@jorisvandenbossche
Copy link
Member Author

@jreback updated this. Added a check to assert_produces_warning that the stacklevel is set correctly, but for now only check it for FutureWarning/DeprecationWarning (all other warnings could be checked as well, but it was a bit too much work to get these all right for now).

@@ -535,7 +535,7 @@ def parse_back_compat(self, w, op=None, value=None):
w, op, value = w
warnings.warn("passing a tuple into Expr is deprecated, "
"pass the where as a single string",
DeprecationWarning)
DeprecationWarning, stacklevel=10)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoosh deep nesting here!

@jreback
Copy link
Contributor

jreback commented Aug 17, 2015

lgtm.

what we actually need is a warning that then traverses the stack and finds the user level code as opposed to using an explict stack level, but I suspect thats a bit tricky.

@jorisvandenbossche
Copy link
Member Author

what we actually need is a warning that then traverses the stack and finds the user level code as opposed to using an explict stack level, but I suspect thats a bit tricky.

Hmm, that goes beyond my python skills, so for somebody else :-) (doubt if this is even possible)

Will merge tomorrow if no further comments

@jreback
Copy link
Contributor

jreback commented Aug 18, 2015

hahha

@jorisvandenbossche
Copy link
Member Author

cc @njsmith since you reported this. This should solve almost all FutureWarnings we have in pandas.

@njsmith
Copy link

njsmith commented Aug 19, 2015

Doing that stack traversal thing is totally doable. Really it should be added as a feature upstream, like warn(..., not_package="pandas.*") to warn at the first level where the code isn't inside pandas (and there's some somewhat-relevant discussion in http://bugs.python.org/file40211). But in the mean time, woot, awesome.

@jorisvandenbossche
Copy link
Member Author

Inferring the stacklevel is indeed not that difficult :-)

Something like this did it:

import warnings
import inspect

frame = inspect.currentframe()
stacklevel = 1

while '\\pandas\\' in frame.f_code.co_filename:
    frame = frame.f_back
    stacklevel += 1

warnings.warn('test!', stacklevel=stacklevel)

(the '\\pandas\\' is because I was testing on Windows)
But don't how robust something like this would be.

@jreback
Copy link
Contributor

jreback commented Aug 19, 2015

FYI I think u have to do a

del frame at the end to break the gc cycles

@jreback
Copy link
Contributor

jreback commented Aug 26, 2015

@jorisvandenbossche let's merge this. I think I already changed a few as I thought you had merged this.

@jreback
Copy link
Contributor

jreback commented Sep 1, 2015

@jorisvandenbossche pls rebase and merge

@jorisvandenbossche jorisvandenbossche force-pushed the stacklevel branch 4 times, most recently from 98a7a40 to 3dbff35 Compare September 3, 2015 13:43
jorisvandenbossche added a commit that referenced this pull request Sep 4, 2015
DEPR: add stacklevel to FutureWarnings (GH9584)
@jorisvandenbossche jorisvandenbossche merged commit aa40dd0 into pandas-dev:master Sep 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants