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

warnings.py gets filename wrong for eval/exec #44793

Closed
Rhamphoryncus mannequin opened this issue Apr 2, 2007 · 14 comments
Closed

warnings.py gets filename wrong for eval/exec #44793

Rhamphoryncus mannequin opened this issue Apr 2, 2007 · 14 comments
Labels
stdlib Python modules in the Lib dir

Comments

@Rhamphoryncus
Copy link
Mannequin

Rhamphoryncus mannequin commented Apr 2, 2007

BPO 1692664
Nosy @gvanrossum, @takluyver
Files
  • python2.6-warningfilename5.diff: Now with more comments!
  • 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 = <Date 2018-04-27.16:39:44.605>
    created_at = <Date 2007-04-02.05:23:18.000>
    labels = ['library']
    title = 'warnings.py gets filename wrong for eval/exec'
    updated_at = <Date 2018-04-27.16:39:44.605>
    user = 'https://bugs.python.org/Rhamphoryncus'

    bugs.python.org fields:

    activity = <Date 2018-04-27.16:39:44.605>
    actor = 'takluyver'
    assignee = 'nnorwitz'
    closed = True
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2007-04-02.05:23:18.000>
    creator = 'Rhamphoryncus'
    dependencies = []
    files = ['7922']
    hgrepos = []
    issue_num = 1692664
    keywords = ['patch']
    message_count = 14.0
    messages = ['52364', '52365', '52366', '52367', '52368', '52369', '52370', '52371', '52372', '52373', '52374', '315843', '315845', '315849']
    nosy_count = 5.0
    nosy_names = ['gvanrossum', 'nnorwitz', 'Rhamphoryncus', 'zseil', 'takluyver']
    pr_nums = []
    priority = 'normal'
    resolution = 'rejected'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue1692664'
    versions = ['Python 2.6']

    @Rhamphoryncus
    Copy link
    Mannequin Author

    Rhamphoryncus mannequin commented Apr 2, 2007

    warnings.warn() gets the filename using the globals' __file__ attribute. When using eval or exec this is often not the context in which the current line was compiled from. The line number is correct, but will be applied to whatever file the globals are from, leading to an unrelated line being printed below the warning.

    The attached patch makes it use caller.f_code.co_filename instead. This also seems to remove the need to normalize .pyc/.pyo files, as well as not needing to use sys.argv[0] for __main__ modules.

    It also cleans up warnings.warn() and adds three unit tests.

    @Rhamphoryncus Rhamphoryncus mannequin closed this as completed Apr 2, 2007
    @Rhamphoryncus Rhamphoryncus mannequin assigned nnorwitz Apr 2, 2007
    @Rhamphoryncus Rhamphoryncus mannequin added the stdlib Python modules in the Lib dir label Apr 2, 2007
    @Rhamphoryncus Rhamphoryncus mannequin closed this as completed Apr 2, 2007
    @Rhamphoryncus Rhamphoryncus mannequin assigned nnorwitz Apr 2, 2007
    @Rhamphoryncus Rhamphoryncus mannequin added the stdlib Python modules in the Lib dir label Apr 2, 2007
    @Rhamphoryncus
    Copy link
    Mannequin Author

    Rhamphoryncus mannequin commented Apr 3, 2007

    The test_stackoverflow_message test I added causes warnings.py to use sys.__warningregistry__ rather than test_warnings.__warningregistry__. This circumvents this self-declared hack of deleting test_warnings.__warningregistry__ to allow regrtest -R to run.

    This version of the patch uses a more general solution to allowing regrtest -R to run. Probably doesn't qualify as a hack anymore...
    File Added: python2.6-warningfilename2.diff

    @Rhamphoryncus
    Copy link
    Mannequin Author

    Rhamphoryncus mannequin commented Apr 4, 2007

    sys._getframe(sys.maxint) produces an OverflowError on 64bit GCC. Patch changed to use sys._getframe(10**9) instead.
    File Added: python2.6-warningfilename3.diff

    @Rhamphoryncus
    Copy link
    Mannequin Author

    Rhamphoryncus mannequin commented Apr 16, 2007

    I've rewritten the unit tests I added to follow the style of walter.doerwald's changes to test_warnings.py (commited to SVN during the same period in which I posted my patch).

    The guard_warnings_registry() context manager is now in test_support.
    File Added: python2.6-warningfilename4.diff

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Apr 23, 2007

    I'm not sure why all the code was in there to begin with it was ancient history. Hopefully Guido remembers. Guido, do you see any particular issue with this approach (ie, removing all the gyrations in warnings and using f_code.co_filename?

    There is a difference in behaviour. If a module modifies its __file__, the old code would pick up the modified version. This new code will find the real file. I'm not sure if that's a bug or a feature though. :-) Similarly with __name__.

    Adam, I like adding the warning_guard using a with statement. Although I don't think comment should be completely removed. Could you add a comment about why the guard is necessary (ie, the repeated calls/-R part)?

    Have you tested with -R and with -u all? ie:

    ./python -E -tt ./Lib/test/regrtest.py -R 4:3:
    and
    ./python -E -tt ./Lib/test/regrtest.py -u all

    (two separate runs).

    In the last test case you are replacing (the old spam7), why does "sys" become "no context"?

    @gvanrossum
    Copy link
    Member

    The main reason for using __file__ instead of the filename baked into the code object is that sometimes .pyc files are moved after being compiled; the code object is not updated, but __file__ has the correct path as it reflects how the module was found during import. Also, code objects may have relative pathnames.

    @Rhamphoryncus
    Copy link
    Mannequin Author

    Rhamphoryncus mannequin commented Apr 23, 2007

    Neal:
    Comment added.

    regrtest -R and -u all report no problems specific to this patch. A clean build of trunk does show problems without it, with test_structmembers, test_tcl, test_telnetlib, test_unicode_file, and test_uuid. At this point I've seen so many weird failures that I think I've started repressing them. ;)

    When no stack frame is available warn() falls back to sys as the module. Since I'm trying to avoid claiming a warning came from a different file, I made it appear to come from <no context> instead. Looking at it now though I wonder if I should go a step further, not having a __warningregistry__ or module at all for such warnings.

    Guido:
    I see what you mean about stale co_filename attributes. I'm not sure what the best fix is now. I'm not exactly enthused at having to learn how the import mechanisms work. ;)
    File Added: python2.6-warningfilename5.diff

    @Rhamphoryncus
    Copy link
    Mannequin Author

    Rhamphoryncus mannequin commented Apr 23, 2007

    I "forgot", test_bsddb3 has problems too, but again they're not related to my patch.

    @nnorwitz
    Copy link
    Mannequin

    nnorwitz mannequin commented Apr 24, 2007

    Unfortunately, test_bsddb3 has lots of problems. :-( They vary by platform and version of bsddb used.

    Given the current code works, I'm not sure it's worth spending time cleaning this up. Especially since I plan to move this code to C. It might be worthwhile to separate out the warning guard. I can't remember if the __warningregistry__ was mucked with in multiple places. If it's only used in the one place, I'm not sure that's worth it either.

    Learning import is a test of manhood. :-)

    @Rhamphoryncus
    Copy link
    Mannequin Author

    Rhamphoryncus mannequin commented Apr 24, 2007

    test_struct is the only thing that touches __warningregistry__ besides test_warning and (in my other patch) test_py3kwarnings. Looks like it would benefit from it, and could use some decorators too..

    def check_float_coerce(format, number):
      ...
    check_float_coerce = with_warning_restore(deprecated_err)

    *cough* Paraphrases of religious quotes relating to the creation of decorators aside, I think that'd be better written as a contextmanager.

    By the time I'm done I'm going to end up with two dozen patch sets covering 80% of the python codebase x_x.

    @zseil
    Copy link
    Mannequin

    zseil mannequin commented Apr 24, 2007

    Bug bpo-1180193 has a patch with a fix for stale co_filename attributes:

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

    @takluyver
    Copy link
    Mannequin

    takluyver mannequin commented Apr 27, 2018

    Time for some bug archaeology!

    The reason given for not using frame.f_code.co_filename for warnings was that it can be wrong when the path of .pyc files changes. However, it looks like that was fixed in bpo-1180193 (thanks for the pointer zseil), so I'd like this idea to be reconsidered.

    Python uses frame.f_code.co_filename for tracebacks and in pdb, so it's counterintuitive that the warnings module works differently. They are all trying to do the same thing: find and show the bit of code that you're interested in.

    This causes problems in IPython with warnings attributed to interactive code: warnings sees it all as part of the __main__ module, so we can't distinguish which input it's pointing to (it's usually obvious, but still...). We have integrated with linecache to make tracebacks work, and I think that if warnings used code.co_filename, it would also work as expected.

    @gvanrossum
    Copy link
    Member

    I recommend opening a new issue (you can link to this one for past discussion).

    @takluyver
    Copy link
    Mannequin

    takluyver mannequin commented Apr 27, 2018

    Thanks Guido, the new issue is bpo-33375.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    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
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant