You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
DeprecationWarnings should be visible by default in interactive usage #8478
DeprecationWarning and PendingDeprecationWarning are invisible by default. The rationale for this is that they are only useful to people who are writing code, so they should not be shown to end-users who are merely running code.
If someone is typing stuff into the interactive REPL or a notebook, though, and the code they type uses some deprecated functionality, then we should actually show them this warning. We know that the author is sitting right there. And they're probably going to take whatever they tested interactively and move it into a more permanent form and ugh.
This problem is particularly acute for packages that are primarily used through the interactive prompt, like numpy. I'm really tired of getting bugs like this: numpy/numpy#5919
The solution is simple: when entering interactive mode, IPython should do something like:
This means "for any DeprecationWarning or PendingDeprecation warning which is caused by code running in the module with name "__main__", use the "default" handler (which means: display once per line of code that triggered it). (Confusingly, the default handler for these warning messages is not "default", but "ignore". So overriding the default with "default" produces a non-default outcome.)
IIRC, the bug was introduced when Python made no DeprecationWarning the default in order to work around the mistake of backporting PyCapsule to 2.8 and raising DeprecationWarning for PyCObject.
I hadn't thought of this, but it's a good idea. I don't think that there's any sensible argument against enabling it for __main__. I'm wondering if we should unhide deprecations across the board, though.
The discussion on python-ideas is a bit depressing.
@takluyver: Right, the PR #8480 currently only shows warnings associated with __main__. But @minrk suggested that maybe we should show all deprecation warnings, even those not associated with __main__.
It is true that warnings are very often attributed incorrectly. The default value for stacklevel= is always wrong, and in the special case whole-module deprecations it turns out that in Python 3 it's not even possible to get right without some major shenanigans. Oops.
I guess most of our imports will happen before we set up the warning filters, so our imports shouldn't be seen.
Though with #8480, even if I install a filter on everything, a user's import imp does not trigger a warning on stderr, probably cause the filter is added too late.
an after-merge + 1
This is one of the reasons we use FutureWarning in pandas instead of DeprecationWarning (although this has of course the disadvantage of also giving warnings from library code you use ..)
@jorisvandenbossche: note that for this to work properly, you also have to make sure the DeprecationWarnings are correctly attributed, which I've noticed that a lot of pandas's aren't. Basically any time you call warnings.warn but don't pass in stacklevel=, then that's a bug.
The backcompat issues are substantial, but you never know, might be worth a
try and see what people say. I probably wouldn't bring it up until a patch
was merged to cpython though, which could then be pointed to as motivation
for getting people to clean up their stacklevels.
On Jun 2, 2015 10:57 AM, "Matthias Bussonnier" [email protected]
wrote:
Should we propose to make stacklevel a mandatory argument in CPython3.6
—
Reply to this email directly or view it on GitHub #8478 (comment).
The backcompat issues are substantial, but you never know, might be worth a
try and see what people say. I probably wouldn't bring it up until a patch
was merged to cpython though, which could then be pointed to as motivation
for getting people to clean up their stacklevels.
I don't get this sentence. My brain is probably fried.
In which case a stacklevel of 1 is correct ? Is stacklevel of 2, The most usefull use value ?
Or should we add a warning mode that do not raise, but actually print the full stacktrace so that you can manually inspect.
@njsmith Yeah, I know you once posted a comment about that somewhere on a pandas issue, but never had the time to look at it.
Do you know of some good explanations what the stacklevel argument exactly does? Because I don't find the docs (https://docs.python.org/2/library/warnings.html#warnings.warn) very helpful to grasp it fully.
@njsmith So actually it is probably rather simple: we need to use stacklevel=2 everywhere, only in cases we have eg a helper function that does the deprecation call which is called in the deprecated function, you need to use stacklevel=3
yep, basically. Little more complicated for warning on "import", but that's a Python bug. And if you use decorators, in which case you need to take into account the wrapper of the decorator.
yes, I was just testing the decorator case (we use this one: https://github.com/pydata/pandas/blob/master/pandas/util/decorators.py#L18), but surprisingly it seems that there the default is ok, and I don't need to specify stacklevel=2, which I don't really understand sorry tested wrongly. stacklevel=2 works as well for the decorator case
Activity
njsmith commentedon May 27, 2015
See also: https://bugs.python.org/issue24294
Carreau commentedon May 27, 2015
We already reset the warning filter in IPython so +1.
We could then turn our shims modules into Dreprecation warning and not User warning also maybe.
charris commentedon May 27, 2015
IIRC, the bug was introduced when Python made no DeprecationWarning the default in order to work around the mistake of backporting PyCapsule to 2.8 and raising DeprecationWarning for PyCObject.
takluyver commentedon May 27, 2015
I think this makes sense. @njsmith , do you have time to do a PR?
Carreau commentedon May 27, 2015
commiting and I push a PR.
Carreau commentedon May 27, 2015
(no test yet though)
minrk commentedon May 27, 2015
I hadn't thought of this, but it's a good idea. I don't think that there's any sensible argument against enabling it for
__main__. I'm wondering if we should unhide deprecations across the board, though.The discussion on python-ideas is a bit depressing.
register default deprecation warning filter.
Carreau commentedon May 27, 2015
if we enable it locally you get too many warning due to deprecation everywhere. Even on 3,4 you get loads of deprecation just because IPython use
imptakluyver commentedon May 27, 2015
But you should only be seeing warnings associated with
__main__, right? So you shouldn't see all the warnings about our use of imp.njsmith commentedon May 28, 2015
@takluyver: Right, the PR #8480 currently only shows warnings associated with
__main__. But @minrk suggested that maybe we should show all deprecation warnings, even those not associated with__main__.It is true that warnings are very often attributed incorrectly. The default value for
stacklevel=is always wrong, and in the special case whole-module deprecations it turns out that in Python 3 it's not even possible to get right without some major shenanigans. Oops.Carreau commentedon May 28, 2015
I guess most of our imports will happen before we set up the warning filters, so our imports shouldn't be seen.
Though with #8480, even if I install a filter on everything, a user's
import impdoes not trigger a warning on stderr, probably cause the filter is added too late.jorisvandenbossche commentedon Jun 2, 2015
an after-merge + 1
This is one of the reasons we use FutureWarning in pandas instead of DeprecationWarning (although this has of course the disadvantage of also giving warnings from library code you use ..)
njsmith commentedon Jun 2, 2015
@jorisvandenbossche: note that for this to work properly, you also have to make sure the DeprecationWarnings are correctly attributed, which I've noticed that a lot of pandas's aren't. Basically any time you call
warnings.warnbut don't pass instacklevel=, then that's a bug.Carreau commentedon Jun 2, 2015
Should we propose to make stacklevel a mandatory argument in CPython3.6
takluyver commentedon Jun 2, 2015
Make an optional argument compulsory because lots of existing code isn't using it? That doesn't sound like a great idea.
Maybe it should issue a warning if you don't pass stacklevel ;-)
njsmith commentedon Jun 2, 2015
The backcompat issues are substantial, but you never know, might be worth a
try and see what people say. I probably wouldn't bring it up until a patch
was merged to cpython though, which could then be pointed to as motivation
for getting people to clean up their stacklevels.
On Jun 2, 2015 10:57 AM, "Matthias Bussonnier" [email protected]
wrote:
njsmith commentedon Jun 2, 2015
That does sound more likely to actually be merged :-).
On Jun 2, 2015 11:01 AM, "Thomas Kluyver" [email protected] wrote:
Carreau commentedon Jun 2, 2015
I don't get this sentence. My brain is probably fried.
In which case a stacklevel of 1 is correct ? Is stacklevel of 2, The most usefull use value ?
Or should we add a warning mode that do not raise, but actually print the full stacktrace so that you can manually inspect.
Thoughts ?
jorisvandenbossche commentedon Jun 2, 2015
@njsmith Yeah, I know you once posted a comment about that somewhere on a pandas issue, but never had the time to look at it.
Do you know of some good explanations what the
stacklevelargument exactly does? Because I don't find the docs (https://docs.python.org/2/library/warnings.html#warnings.warn) very helpful to grasp it fully.jorisvandenbossche commentedon Jun 2, 2015
@njsmith So actually it is probably rather simple: we need to use
stacklevel=2everywhere, only in cases we have eg a helper function that does the deprecation call which is called in the deprecated function, you need to usestacklevel=3Carreau commentedon Jun 2, 2015
yep, basically. Little more complicated for warning on "import", but that's a Python bug. And if you use decorators, in which case you need to take into account the wrapper of the decorator.
jorisvandenbossche commentedon Jun 2, 2015
yes, I was just testing the decorator case (we use this one: https://github.com/pydata/pandas/blob/master/pandas/util/decorators.py#L18),
but surprisingly it seems that there the default is ok, and I don't need to specifysorry tested wrongly.stacklevel=2, which I don't really understandstacklevel=2works as well for the decorator caseCarreau commentedon Jun 2, 2015
Yah I was thinking of
Calculate correct stack level for deprecation warnings to show them t…
Calculate correct stack level for deprecation warnings to show them t…