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

Suboptimal stacklevel of deprecation warnings for formatter and imp modules #67998

Closed
Arfrever mannequin opened this issue Mar 30, 2015 · 11 comments
Closed

Suboptimal stacklevel of deprecation warnings for formatter and imp modules #67998

Arfrever mannequin opened this issue Mar 30, 2015 · 11 comments
Assignees
Labels
deferred-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@Arfrever
Copy link
Mannequin

Arfrever mannequin commented Mar 30, 2015

BPO 23810
Nosy @brettcannon, @ncoghlan, @larryhastings, @ericsnowcurrently, @berkerpeksag, @serhiy-storchaka
Superseder
  • bpo-24305: The new import system makes it inconvenient to correctly issue a deprecation warning for a module
  • Files
  • better_stacklevel.diff
  • deprecated_module_stacklevel.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/brettcannon'
    closed_at = <Date 2015-08-14.18:18:33.979>
    created_at = <Date 2015-03-30.01:54:14.679>
    labels = ['deferred-blocker', 'type-bug', 'library']
    title = 'Suboptimal stacklevel of deprecation warnings for formatter and imp modules'
    updated_at = <Date 2015-08-14.18:18:57.969>
    user = 'https://bugs.python.org/Arfrever'

    bugs.python.org fields:

    activity = <Date 2015-08-14.18:18:57.969>
    actor = 'brett.cannon'
    assignee = 'brett.cannon'
    closed = True
    closed_date = <Date 2015-08-14.18:18:33.979>
    closer = 'brett.cannon'
    components = ['Library (Lib)']
    creation = <Date 2015-03-30.01:54:14.679>
    creator = 'Arfrever'
    dependencies = []
    files = ['39196', '39550']
    hgrepos = []
    issue_num = 23810
    keywords = ['patch']
    message_count = 11.0
    messages = ['239554', '239607', '239610', '241944', '241968', '242015', '244260', '244304', '244396', '246285', '248600']
    nosy_count = 7.0
    nosy_names = ['brett.cannon', 'ncoghlan', 'larry', 'Arfrever', 'eric.snow', 'berker.peksag', 'serhiy.storchaka']
    pr_nums = []
    priority = 'deferred blocker'
    resolution = 'duplicate'
    stage = 'patch review'
    status = 'closed'
    superseder = '24305'
    type = 'behavior'
    url = 'https://bugs.python.org/issue23810'
    versions = ['Python 3.5', 'Python 3.6']

    @Arfrever
    Copy link
    Mannequin Author

    Arfrever mannequin commented Mar 30, 2015

    https://hg.python.org/cpython/rev/2a336cc29282 changed stacklevel of some deprecation warnings.
    However new value is still not useful, because either _frozen_importlib or importlib/_bootstrap.py is now mentioned in deprecation warnings:

    $ cat test.py
    import formatter
    import imp
    $ python3.4 -Wd test.py
    /usr/lib64/python3.4/formatter.py:24: PendingDeprecationWarning: the formatter module is deprecated and will be removed in Python 3.6
      'Python 3.6', PendingDeprecationWarning)
    /usr/lib64/python3.4/imp.py:32: PendingDeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
      PendingDeprecationWarning)
    $ python3.5 -Wd test.py
    _frozen_importlib:321: DeprecationWarning: the formatter module is deprecated and will be removed in Python 3.6
    /usr/lib64/python3.5/importlib/_bootstrap.py:321: PendingDeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
      return f(*args, **kwds)

    @Arfrever Arfrever mannequin assigned brettcannon Mar 30, 2015
    @Arfrever Arfrever mannequin added the stdlib Python modules in the Lib dir label Mar 30, 2015
    @brettcannon
    Copy link
    Member

    Probably need to introduce a new keyword argument just for deprecated imports or some helper function in importlib to get the stack depth right (else there is a risk of breaking the stack depth in any minor release whenever importlib's depth shifts). Something like the following should be enough (obviously done in warnings instead of per-file):

    try:
    level = 1
    while True:
    frame = sys._getframe(level)
    print(frame.f_code.co_filename)
    if '_bootstrap' not in frame.f_code.co_filename:
    break
    level += 1
    except ValueError:
    pass
    print(sys._getframe(2).f_code.co_filename)
    warnings.warn("the imp module is deprecated in favour of importlib; "
    "see the module's documentation for alternative uses",
    PendingDeprecationWarning, stacklevel=level+1)

    Otherwise the depths should just go back to what they were at.

    @brettcannon brettcannon added release-blocker type-bug An unexpected behavior, bug, or error labels Mar 30, 2015
    @serhiy-storchaka
    Copy link
    Member

    Similar feature is needed for warnings in the re module. Methods of regular expression pattern can be called directly or from module-level wrappers. In these case the stack level differs by 1. And sometimes warnings are emitted in recursive parser, when stack level is variable.

    @brettcannon
    Copy link
    Member

    Here is a private function in warnings for calculating the stack depth to the first frame not referencing some key string. Can someone look at it to make sure it looks reasonable?

    I'm starting with it being private since it uses sys._getframe() and I don't know how widely needed it is.

    @serhiy-storchaka
    Copy link
    Member

    Unfortunately this will not help for re, because the trace passes through three files: Lib/sre_parse.py, Lib/sre_compile.py and Lib/re.py.

    @brettcannon
    Copy link
    Member

    OK, I'll look at making it more general for multiple names to skip.

    @ncoghlan
    Copy link
    Contributor

    bpo-24305 covers either making this a public API for general use, or else just making module level deprecation warnings skip the import machinery automatically.

    I also wonder whether Eric's _boostrap_external changes might have broken any of the frame hiding tricks for tracebacks.

    @ericsnowcurrently
    Copy link
    Member

    I had a similar concern, Nick, but don't think I did anything that would have broken the frame hiding logic. That said, I did not take stacklevel for warnings into account.

    @brettcannon
    Copy link
    Member

    Latest patch should work for Serhiy's needs by taking a container of names to compare against the filename instead of a single argument.

    @larryhastings
    Copy link
    Contributor

    This regression isn't thrilling, but it's not the kind of "OMG we can't release with this bug" level of escalation I associate with an actual release blocker. Let's at least defer it for now, and maybe we'll even reduce it further later.

    @brettcannon
    Copy link
    Member

    Merging with the other issue so there is a single place to track this

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    jasongrout added a commit to jasongrout/ipywidgets that referenced this issue Aug 26, 2022
    …o the user.
    
    Generating deprecation warnings with the correct stacklevel is tricky to get right. Up to now, users have not been seeing these deprecation warnigns because they had incorrect stacklevels. See python/cpython#68493 and python/cpython#67998 for good discussions, as well as ipython/ipython#8478 (comment) and ipython/ipython#8480 (comment) for more context.
    
    We see these issues directly. Normally stacklevel=2 might be enough to target the caller of a function. However, in our deprecation warning in the __init__ method, each level of subclasses adds one stack frame, so depending on the subclass, the appropriate stacklevel is different. Thus this PR implements a trick from a discussion in Python to calculate the first stack frame that is external to the ipywidgets library, and use that as the target stack frame for the deprecation warning.
    jasongrout added a commit to jasongrout/ipywidgets that referenced this issue Aug 26, 2022
    …o the user.
    
    Generating deprecation warnings with the correct stacklevel is tricky to get right. Up to now, users have not been seeing these deprecation warnigns because they had incorrect stacklevels. See python/cpython#68493 and python/cpython#67998 for good discussions, as well as ipython/ipython#8478 (comment) and ipython/ipython#8480 (comment) for more context.
    
    We see these issues directly. Normally stacklevel=2 might be enough to target the caller of a function. However, in our deprecation warning in the __init__ method, each level of subclasses adds one stack frame, so depending on the subclass, the appropriate stacklevel is different. Thus this PR implements a trick from a discussion in Python to calculate the first stack frame that is external to the ipywidgets library, and use that as the target stack frame for the deprecation warning.
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    deferred-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants