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

Improve AttributeError message for partially initialized module #77418

Closed
serhiy-storchaka opened this issue Apr 6, 2018 · 8 comments
Closed
Labels
3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 33237
Nosy @brettcannon, @ncoghlan, @ericsnowcurrently, @serhiy-storchaka
PRs
  • bpo-33237: Improve AttributeError message for partially initialized module. #6398
  • 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-10-30.11:21:29.153>
    created_at = <Date 2018-04-06.15:43:32.578>
    labels = ['interpreter-core', 'type-feature', '3.8']
    title = 'Improve AttributeError message for partially initialized module'
    updated_at = <Date 2018-10-30.11:21:29.152>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2018-10-30.11:21:29.152>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-10-30.11:21:29.153>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2018-04-06.15:43:32.578>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 33237
    keywords = ['patch']
    message_count = 8.0
    messages = ['315019', '315020', '315071', '315076', '315136', '315166', '328131', '328895']
    nosy_count = 4.0
    nosy_names = ['brett.cannon', 'ncoghlan', 'eric.snow', 'serhiy.storchaka']
    pr_nums = ['6398']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue33237'
    versions = ['Python 3.8']

    @serhiy-storchaka
    Copy link
    Member Author

    Cyclic import usually leads to an AttributeError "module 'spam' has no attribute 'ham'" which usually is confusing because in normal case 'spam.ham' exists, and the user can have no ideas why it is disappeared.

    The proposed PR allows to specialize the AttributeError message for partially initialized module. Any suggestions about the error message?

    @serhiy-storchaka serhiy-storchaka added 3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Apr 6, 2018
    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Apr 6, 2018

    While I like the idea of this change, the "partially initialized" addition is fairly subtle, and relatively easy to miss.

    Perhaps append "(most likely due to a circular import)" to the partially initialized case?:

    AttributeError: partially initialized "module 'spam' has no attribute 'ham' (most likely due to a circular import).
    

    Crucially, for folks encountering the error for the first time, that also introduces them to the main phrase they may want to search for: "circular import".

    The "most likely" weasel wording stems from the fact that the problem won't always be with the circular import - they may have just straight up referenced the wrong module or the wrong attribute name, so the apparently circular import is an error.

    @brettcannon
    Copy link
    Member

    +1 from me for Nick's suggestion.

    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Apr 8, 2018

    Oops, just realised my suggested text had an extraneous double quote in it due to a copy-and-paste error. Fixed version:

    AttributeError: partially initialized module 'spam' has no attribute 'ham' (most likely due to a circular import).
    

    @serhiy-storchaka
    Copy link
    Member Author

    I have applied the Nick's suggestion. Needed to find a place for new test.

    The code is copied from PyImport_ImportModuleLevelObject(). I'm not happy from how verbose it is. And testing mod.__spec__._initialized adds relatively large overhead for importing already imported module in PyImport_ImportModuleLevelObject(). Is it possible to invent a faster way for checking whether the module is partially imported?

    @ncoghlan
    Copy link
    Contributor

    The main idea that comes to mind is to cache a reference to _frozen_importlib._module_locks in the interpreter state, and do a key lookup in there (since any in-progress import should have a lock allocated to it).

    That would be a separate performance issue though - for this issue, we're on an error handling path, so the speed with which the error gets reported isn't critical (although it does technically slow down try/except import fallback chains).

    @serhiy-storchaka
    Copy link
    Member Author

    Example:

    $ cat foo.py
    import bar
    bar.baz
    $ cat bar.py
    import foo
    baz = 2
    $ ./python foo.py 
    Traceback (most recent call last):
      File "foo.py", line 1, in <module>
        import bar
      File "/home/serhiy/py/cpython/bar.py", line 1, in <module>
        import foo
      File "/home/serhiy/py/cpython/foo.py", line 2, in <module>
        bar.baz
    AttributeError: module 'bar' has no attribute 'baz'

    Patched:

    $ ./python foo.py 
    Traceback (most recent call last):
      File "foo.py", line 1, in <module>
        import bar
      File "/home/serhiy/py/cpython/bar.py", line 1, in <module>
        import foo
      File "/home/serhiy/py/cpython/foo.py", line 2, in <module>
        bar.baz
    AttributeError: partially initialized module 'bar' has no attribute 'baz' (most likely due to a circular import)

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 3e429dc by Serhiy Storchaka in branch 'master':
    bpo-33237: Improve AttributeError message for partially initialized module. (GH-6398)
    3e429dc

    @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
    3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants