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

Inspect.getsource raises wrong error on classes in interactive session #88814

Closed
akulakov opened this issue Jul 15, 2021 · 12 comments
Closed

Inspect.getsource raises wrong error on classes in interactive session #88814

akulakov opened this issue Jul 15, 2021 · 12 comments
Labels
3.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error

Comments

@akulakov
Copy link
Contributor

BPO 44648
Nosy @jaraco, @aroberge, @ambv, @pablogsal, @miss-islington, @FFY00, @akulakov
PRs
  • bpo-44648: Fix error type in inspect.getsource() in interactive session #27171
  • [3.10] bpo-44648: Fix error type in inspect.getsource() in interactive session (GH-27171) #27495
  • bpo-44771: rename namespacedata01 to namespacedata #27484
  • bpo-44771: Apply changes from importlib_resources 5.2.1 #27436
  • 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 2021-07-30.17:47:14.167>
    created_at = <Date 2021-07-15.18:31:38.611>
    labels = ['type-bug', '3.10', '3.11']
    title = 'Inspect.getsource raises wrong error on classes in interactive session'
    updated_at = <Date 2021-08-02.09:59:39.832>
    user = 'https://github.com/akulakov'

    bugs.python.org fields:

    activity = <Date 2021-08-02.09:59:39.832>
    actor = 'pablogsal'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-07-30.17:47:14.167>
    closer = 'lukasz.langa'
    components = []
    creation = <Date 2021-07-15.18:31:38.611>
    creator = 'andrei.avk'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 44648
    keywords = ['patch']
    message_count = 11.0
    messages = ['397573', '397629', '397631', '397634', '397636', '398579', '398580', '398588', '398589', '398590', '398748']
    nosy_count = 8.0
    nosy_names = ['jaraco', 'aroberge', 'lukasz.langa', 'pablogsal', 'miss-islington', 'FFY00', 'andrei.avk', 'othmaneelbouri1']
    pr_nums = ['27171', '27495', '27484', '27436']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue44648'
    versions = ['Python 3.10', 'Python 3.11']

    @akulakov
    Copy link
    Contributor Author

    [ins] In [63]: class A:pass

    [ins] In [64]: import inspect

    [ins] In [65]: inspect.getsource(A)

    [snip]

    /usr/local/Cellar/python@3.9/3.9.1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/inspect.py in getfile(object)
    664 if getattr(module, '__file__', None):
    665 return module.__file__
    --> 666 raise TypeError('{!r} is a built-in class'.format(object))

    The error is 'X is a built-in class', instead it should be OSError, source code is not available.

    @akulakov akulakov added 3.11 only security fixes type-bug An unexpected behavior, bug, or error 3.9 only security fixes 3.10 only security fixes labels Jul 15, 2021
    @ambv
    Copy link
    Contributor

    ambv commented Jul 16, 2021

    Why do you think OSError fits here? Since objects provided on the command line by definition cannot have source code files, the problem isn't that the file is missing or inaccessible. Rather, the value provided to getsource() is wrong. So, in my view, this should rather be ValueError.

    Moreover, checking for __main__ is insufficient as there are __main__.py modules after all in many packages.

    @akulakov
    Copy link
    Contributor Author

    Łukasz:

    Thanks for looking at this!

    • I agree OSError is not ideal, but I chose it because it's consistent with how inspect reports similar errors. For example, see all instances of OSError, except for first, in this function:

    def findsource(object):

    I think it's an implementation detail that class source is not available on interactive prompt, for example function source is available via inspect.getsource().

    If a user can do getsource(func) and getsource(cls) in a file, and getsource(func) on interactive prompt, it seems wrong to return a ValueError when the user is trying to getsource(cls) in interactive.

    I'm open to change it to ValueError though because I think the err message here is more important to avoid user confusion.

    • good point about __main__.py files, I didn't remember about them. However they will have the __file__ attribute set, so they will return that right above the check for '__main__' in my patch.

    I tested it to make sure:
    import inspect
    class A: 0
    print(inspect.getsource(A))

    ./python.exe ~/temp/main.py
    class A: 0

    @akulakov
    Copy link
    Contributor Author

    I'm not sure though how is the unit test succeeding since the test module should have __file__ set. Looking into it..

    @akulakov
    Copy link
    Contributor Author

    This is kind of interesting:

    • The unit test was wrong, it was catching the wrong OSError. (I was catching regex first but after some tweaking and changes I lost it and forgot to readd)

    • The reason it was passing is exactly what you pointed out -- the main.py in unittest package. After I change the class' module to main in the test, it starts looking at sys.modules['main'] which is the one in unittest, which doesn't have this class, which causes the inspect to throw another OSError.

    • I fixed the unit test by both checking for error regex and patching sys.modules['__main__'] (and of course restoring it later).

    Thanks for helping to catch this :-)

    @ambv
    Copy link
    Contributor

    ambv commented Jul 30, 2021

    New changeset 48a6255 by andrei kulakov in branch 'main':
    bpo-44648: Fix error type in inspect.getsource() in interactive session (GH-27171)
    48a6255

    @ambv
    Copy link
    Contributor

    ambv commented Jul 30, 2021

    Since this is an exception type change, I'd feel more comfortable leaving 3.9 out of the backports here. That way it will be easier for application authors to differentiate between when the change was introduced or not.

    @ambv ambv removed 3.9 only security fixes labels Jul 30, 2021
    @othmaneelbouri1
    Copy link
    Mannequin

    othmaneelbouri1 mannequin commented Jul 30, 2021

    ince this is an exception type change, I'd feel more comfortable leaving 3.9 out of the backports here. That way it will be easier for application authors to differentiate between when the change was introduced or not.

    @ambv
    Copy link
    Contributor

    ambv commented Jul 30, 2021

    New changeset f468ede by Miss Islington (bot) in branch '3.10':
    bpo-44648: Fix error type in inspect.getsource() in interactive session (GH-27171) (GH-27495)
    f468ede

    @ambv
    Copy link
    Contributor

    ambv commented Jul 30, 2021

    Thanks, Andrei! ✨ 🍰 ✨

    @ambv ambv closed this as completed Jul 30, 2021
    @ambv ambv closed this as completed Jul 30, 2021
    @pablogsal
    Copy link
    Member

    Unfortunately, this PR has broken test_inspect when executed with -R, see:

    https://bugs.python.org/issue44808

    @serhiy-storchaka
    Copy link
    Member

    I am not sure that OSError was a correct exception. Before this, the only exception raised by inspect.getfile() and inspect.getsourcefile() was TypeError. In particularly this change cause an issue in doctests (#113329).

    Even if this change is correct, it should be documented, have versionchanged directives and a What's New entry. But maybe changing it to TypeError is better in long term.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants