Skip to content

DeprecationWarnings should be visible by default in interactive usage #8478

Closed

Description

@njsmith

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:

warnings.filterwarnings("default", category=DeprecationWarning, module="__main__")
warnings.filterwarnings("default", category=PendingDeprecationWarning, module="__main__")

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.)

Activity

  1. njsmith commented on May 27, 2015

    @njsmith
    ContributorAuthor
  2. Carreau commented on May 27, 2015

    @Carreau
    Member

    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.

  3. charris commented on May 27, 2015

    @charris

    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.

  4. takluyver commented on May 27, 2015

    @takluyver
    Member

    I think this makes sense. @njsmith , do you have time to do a PR?

  5. Carreau commented on May 27, 2015

    @Carreau
    Member

    commiting and I push a PR.

  6. Carreau commented on May 27, 2015

    @Carreau
    Member

    (no test yet though)

  7. minrk commented on May 27, 2015

    @minrk
    Member

    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.

  8. added a commit that references this issue on May 27, 2015
    1478e0d
  9. Carreau commented on May 27, 2015

    @Carreau
    Member

    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 imp

  10. takluyver commented on May 27, 2015

    @takluyver
    Member

    But you should only be seeing warnings associated with __main__, right? So you shouldn't see all the warnings about our use of imp.

  11. njsmith commented on May 28, 2015

    @njsmith
    ContributorAuthor

    @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.

  12. Carreau commented on May 28, 2015

    @Carreau
    Member

    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.

  13. jorisvandenbossche commented on Jun 2, 2015

    @jorisvandenbossche
    Contributor

    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 ..)

  14. njsmith commented on Jun 2, 2015

    @njsmith
    ContributorAuthor

    @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.

  15. Carreau commented on Jun 2, 2015

    @Carreau
    Member

    Should we propose to make stacklevel a mandatory argument in CPython3.6

  16. takluyver commented on Jun 2, 2015

    @takluyver
    Member

    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 ;-)

  17. njsmith commented on Jun 2, 2015

    @njsmith
    ContributorAuthor

    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).

  18. njsmith commented on Jun 2, 2015

    @njsmith
    ContributorAuthor

    That does sound more likely to actually be merged :-).
    On Jun 2, 2015 11:01 AM, "Thomas Kluyver" [email protected] wrote:

    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 ;-)

    —
    Reply to this email directly or view it on GitHub
    #8478 (comment).

  19. Carreau commented on Jun 2, 2015

    @Carreau
    Member

    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.

    Thoughts ?

  20. jorisvandenbossche commented on Jun 2, 2015

    @jorisvandenbossche
    Contributor

    @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.

  21. jorisvandenbossche commented on Jun 2, 2015

    @jorisvandenbossche
    Contributor

    @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

  22. Carreau commented on Jun 2, 2015

    @Carreau
    Member

    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.

  23. jorisvandenbossche commented on Jun 2, 2015

    @jorisvandenbossche
    Contributor

    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

  24. Carreau commented on Jun 2, 2015

    @Carreau
    Member

    Yah I was thinking of

    @dec 
    fun():
       warn(staclevel=probably(3))
    
  25. added this to the 4.0 milestone on Aug 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions