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

Enhance frame handing in warnings.warn() to skip by module rather that stacklevel #39615

Closed
doerwalter opened this issue Nov 27, 2003 · 11 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@doerwalter
Copy link
Contributor

doerwalter commented Nov 27, 2003

BPO 850482
Nosy @doerwalter, @devdanzin, @jayvdb
Files
  • diff.txt
  • diff2.txt
  • 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 = None
    closed_at = None
    created_at = <Date 2003-11-27.23:12:08.000>
    labels = ['type-feature', 'library']
    title = 'Enhance frame handing in warnings.warn()'
    updated_at = <Date 2015-12-10.08:16:31.826>
    user = 'https://github.com/doerwalter'

    bugs.python.org fields:

    activity = <Date 2015-12-10.08:16:31.826>
    actor = 'jayvdb'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2003-11-27.23:12:08.000>
    creator = 'doerwalter'
    dependencies = []
    files = ['5709', '5710']
    hgrepos = []
    issue_num = 850482
    keywords = ['patch']
    message_count = 6.0
    messages = ['44953', '44954', '44955', '44956', '82016', '256170']
    nosy_count = 4.0
    nosy_names = ['doerwalter', 'zseil', 'ajaksu2', 'jayvdb']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue850482'
    versions = ['Python 3.2']

    Linked PRs

    @doerwalter
    Copy link
    Contributor Author

    This patch enhances warnings.warn() in the following
    way: The stacklevel passed in may be negative. In that
    case the call stack is searched for the innermost frame
    whose module name differs in the first -stacklevel
    components. This frame will be used in the report.

    So when you have the following call stack:
    m1.f()
    m1.m11.f()
    m1.m12.f()
    m2.m21.f()
    m2.m22.f()

    and the innermost function() m2.m22.f() call
    warnings.warn() with a stacklevel of -2 the frame
    reported will be from m1.m12.f(), because it is the
    first one from outside the m2 package.

    @doerwalter doerwalter added stdlib Python modules in the Lib dir labels Nov 27, 2003
    @zseil
    Copy link
    Mannequin

    zseil mannequin commented Apr 2, 2007

    I don't know how desireable this feature is, but
    I think that a better solution would be to add
    a new parameter to warnings.warn(), instead of
    reusing the old one.

    Also, your description is wrong; when warn() in
    your example is called with stacklevel -2, the
    reported frame is m2.m21.f(). To get your result,
    you have to call warn() with stacklevel -1.

    I think that a better solution would be to add
    a skipmodules='moduleprefix' parameter to the
    warn() function. warn() would then simply look
    for the first frame whose module name doesn't
    start with this prefix.

    @doerwalter
    Copy link
    Contributor Author

    Here's an updated patch that implements your suggestion: skipmodules is a regular expression. Any module whose name matches the RE will be skipped when determining the stacklevel.
    File Added: diff2.txt

    @zseil
    Copy link
    Mannequin

    zseil mannequin commented Apr 15, 2007

    Looks good to me. I like that skipmodules is a
    regular expression, but I don't know how will it
    affect the intended rewrite in C:

    http://www.python.org/sf/1631171

    It would be nice if the documentation mentioned
    what is the interaction between stacklevel and
    skipmodules parameters, at least something like:

    "If you want to issue a warning outside your module,
    and you don't know how many frames will have to be
    skipped for that, pass some big integer for stacklevel"

    If this patch is accepted, it should be put to use in
    the _struct module:

    >>> struct.Struct("<l").pack(4023029188) # note: module is sys
    sys:1: DeprecationWarning: struct integer overflow masking is deprecated

    @devdanzin
    Copy link
    Mannequin

    devdanzin mannequin commented Feb 14, 2009

    Patch has tests, needs updating.

    @devdanzin devdanzin mannequin added type-feature A feature request or enhancement labels Feb 14, 2009
    @jayvdb
    Copy link
    Mannequin

    jayvdb mannequin commented Dec 10, 2015

    See similar http://bugs.python.org/issue25216 aka #69403

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
    @gpshead gpshead changed the title Enhance frame handing in warnings.warn() Enhance frame handing in warnings.warn() to skip by module rather that stacklevel Jan 1, 2023
    @gpshead gpshead self-assigned this Jan 1, 2023
    @gpshead
    Copy link
    Member

    gpshead commented Jan 1, 2023

    Assigning to me to see if I get to this as I'm really wanting this feature while trying to add a warning deep within an an complicated package with multiple public API surfaces at different stacklevels.

    gpshead added a commit to gpshead/cpython that referenced this issue Jan 8, 2023
    `warnings.warn()` gains the ability to skip stack frames based on code
    filename prefix rather than only a numeric `stacklevel=` via a new
    `skip_file_prefixes=` keyword argument.
    @merwok
    Copy link
    Member

    merwok commented Jan 8, 2023

    Skipping by module seems good, but the implementation relies on file names — could it be module or package name?

    @gpshead
    Copy link
    Member

    gpshead commented Jan 8, 2023

    It seems like "by fully qualified module name" is what code authors might want as that's how we think... But in practice that might be hard to implement in a reliable manner. The __file__ module or package prefix match effectively lets you do that. Anticipated common use path prefix matching use cases: skip_file_prefixes=(__file__,) or skip_file_prefixes=(os.path.dirname(__file__),)

    Investigating... The code objects in the frame appears to have a .co_qualname attribute, but it that always set?
    Code objects are always defined within a file thus co_filename should be set for anything coming from a .py source file in a module or package (C extensions or manual code object creation could supply whatever "filename" they want, but those unusual things aren't cases I'd worry about matching and skipping), but code objects are not required to have names or qualified names (lambdas, comprehension expressions, nested functions, etc)...

    It might be possible to get the base module name itself via frame.f_globals['__name__'] but even that sounds unreliable, and using just the __name__ as a match to skip is pretty non-specific.

    I'll poke around, but I'm not sure if it is possible.

    The other constraint I've been keeping in this code: It must be a fast check. warn() should not do a lot of computation.

    @gpshead
    Copy link
    Member

    gpshead commented Jan 12, 2023

    I don't believe module or package name is possible. Indeed frame.f_code.co_qualname doesn't give a module or package name. Frames and code objects don't have any reference back to a module or namespace. So I don't see any easy, fast, and reliable way to do that beyond the filename based match as my PR implements.

    gpshead added a commit that referenced this issue Jan 28, 2023
    `warnings.warn()` gains the ability to skip stack frames based on code
    filename prefix rather than only a numeric `stacklevel=` via a new
    `skip_file_prefixes=` keyword argument.
    iritkatriel added a commit to iritkatriel/cpython that referenced this issue Jan 29, 2023
    mdboom pushed a commit to mdboom/cpython that referenced this issue Jan 31, 2023
    …n#100840)
    
    `warnings.warn()` gains the ability to skip stack frames based on code
    filename prefix rather than only a numeric `stacklevel=` via a new
    `skip_file_prefixes=` keyword argument.
    @gpshead
    Copy link
    Member

    gpshead commented Feb 8, 2023

    I'm calling this fixed as it can skip by filename now which was a performant way to implement this.

    @gpshead gpshead closed this as completed Feb 8, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants