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

AttributeError message text should include module name #52544

Closed
cjerdonek opened this issue Apr 3, 2010 · 15 comments
Closed

AttributeError message text should include module name #52544

cjerdonek opened this issue Apr 3, 2010 · 15 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@cjerdonek
Copy link
Member

BPO 8297
Nosy @amauryfa, @devdanzin, @ezio-melotti, @bitdancer, @durban, @cjerdonek, @ethanfurman
Files
  • issue_8297.diff: Patch for trunk
  • issue_8297_test.py: Patch for trunk
  • issue_8297.diff: patch against py3k
  • issue8297.stoneleaf.01.patch
  • 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/ethanfurman'
    closed_at = <Date 2014-04-24.23:48:55.905>
    created_at = <Date 2010-04-03.05:30:05.742>
    labels = ['interpreter-core', 'type-feature']
    title = 'AttributeError message text should include module name'
    updated_at = <Date 2014-04-25.00:14:09.602>
    user = 'https://github.com/cjerdonek'

    bugs.python.org fields:

    activity = <Date 2014-04-25.00:14:09.602>
    actor = 'ethan.furman'
    assignee = 'ethan.furman'
    closed = True
    closed_date = <Date 2014-04-24.23:48:55.905>
    closer = 'benjamin.peterson'
    components = ['Interpreter Core']
    creation = <Date 2010-04-03.05:30:05.742>
    creator = 'chris.jerdonek'
    dependencies = []
    files = ['16780', '16815', '18124', '34634']
    hgrepos = []
    issue_num = 8297
    keywords = ['patch']
    message_count = 15.0
    messages = ['102224', '102229', '102232', '102463', '102514', '102549', '102598', '111153', '111171', '111200', '111232', '214434', '214936', '217139', '217142']
    nosy_count = 9.0
    nosy_names = ['amaury.forgeotdarc', 'ajaksu2', 'ezio.melotti', 'r.david.murray', 'daniel.urban', 'chris.jerdonek', 'ysj.ray', 'ethan.furman', 'python-dev']
    pr_nums = []
    priority = 'low'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue8297'
    versions = ['Python 3.5']

    @cjerdonek
    Copy link
    Member Author

    It would be nice if the error message for an AttributeError could include the module name when getting from a module -- just like it does for getting from a class.

    This would make the message more helpful. For example, it would help in diagnosing issues like the ones mentioned in this report:

    http://bugs.python.org/issue7559

    EXAMPLE (using latest from trunk Python 2.7a4+):

    import sys
    
    class TestClass(object):
        pass
    
    m = sys
    c = TestClass

    print "CLASS: %s" % c

    try:
    c.asdf
    except AttributeError, err:
    print err

    print "\nMODULE: %s" % m

    try:
    m.adsf
    except AttributeError, err:
    print err


    OUTPUT:

    CLASS: <class '__main__.TestClass'>
    type object 'TestClass' has no attribute 'asdf'

    MODULE: <module 'sys' (built-in)>
    'module' object has no attribute 'adsf'


    The latter message could instead be (paralleling the text in the case of a class)--

    module object 'sys' has no attribute 'adsf'

    @cjerdonek cjerdonek added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Apr 3, 2010
    @ysjray
    Copy link
    Mannequin

    ysjray mannequin commented Apr 3, 2010

    Yes, I agree with this feature request. And also the super(Class, obj) call should return a reasonable AttributeError message when the requested attribute is not found in one of Class's base classes, not just 'super' object has no attribute 'xxx'.

    @ysjray
    Copy link
    Mannequin

    ysjray mannequin commented Apr 3, 2010

    In fact, there are only three types of tp_getattro functions:
    1.For type objects, it is type_getattro(), in case of AttributeError, this function give the message format:
    type object %(type)s has no attribute %(attr)s
    2.For super objects, it is super_getattro(), in case of no attribute found in one of its base class, it calls the 3'th getattr function below.
    3.For the base type 'object' and other new style class, it is PyObject_GenericGetAttr(), and in case of AttributeError, this function give the message format:
    %(type)s object has no attribute %(attr)s

    So, there are only tow formats of AttributeError's exception messages:
    1.type object %(type)s has no attribute %(attr)s
    2.%(type)s object has no attribute %(attr)s
    The first one is for type objects, the second one is for all the instances objects.

    In most cases, these tow formats it is enough for program to display, bu t it is not well enough. Take the module objects for example, in case of AttributeError, we will always hope to know exactly which module has no attribute, not only the message: 'module object has attribute xxx'.

    Also for the super() call, take super(A, b).xxx() for example, if the attribute is not found in the class next to the A in b's type's mro list, PyObject_GenericGetAttr() will be called with the two arguments, the super object its self and 'xxx'. But there are only few valid attributes of a super object, like '__thisclass__', '__self__', '__self_class__'. In most cases, we don't need these attributes of a super object, what we need is the attribute of one of b's type's base types. So I think once the AttributeError is raised in PyObject_GenericGetAttr() call in the end of super_getattro(), the exception message should tell us in which base class python can not found the attribte 'xxx', but not the super object itself, although the exception is raised in the PyObject_GenericGetAttr(<super obj>, 'xxx').

    For the solution, I think the type_getattro and super_getattro can just return NULL to indicate the attribute is not found, but for a wrapper function of this tow which is the tp_getattro for each type to raise the attribute error, with the reasonable exception message. For example, mudule type can tell which module has no attribute, super type can tell which base class has no attribute, and so on.

    What about others' opinion?

    @ysjray
    Copy link
    Mannequin

    ysjray mannequin commented Apr 6, 2010

    This patch makes the AttributeError message generated from getting attributes from module object more helpful, that is, print the module name. Now the error message is:
    module object 'mod_name' has no attribute 'xxx'
    Instead of:
    'module' object has no attribute 'xxx'

    In this patch, I add a function PyObject_GenericFindAttr(), which is almost the same as PyObject_GenericGetAttr(), except that return NULL instead of setting the exception, and make PyObject_GenericGetAttr() call this function and setting exception in the case of NULL return value. Through this, each type can custom its own AttributeError message, while using the uniform process of getting the attribute in PyObject_GenericFindAttr(). For example, PyModuleType's tp_getattro is set to a new function PyModule_GetAttr(), which calls the PyObject_GenericFindAttr() can set the customed AttributeError message.

    Besides, I think the super object's AttributeError message should also be improved, but I didn't include that in the patch. I don't know weather it is suitable.

    @cjerdonek
    Copy link
    Member Author

    Great -- thanks a lot for taking a stab at this!

    @bitdancer
    Copy link
    Member

    This is a feature request, so it only applies to unreleased versions. It'll be up to Benjamin whether it can go into 2.7, if the change is approved, since 2.7 is now technically in feature freeze.

    The patch also needs unit tests.

    I do like the idea of making the error messages more specific. I have not reviewed the patch itself.

    @bitdancer bitdancer added interpreter-core (Objects, Python, Grammar, and Parser dirs) and removed stdlib Python modules in the Lib dir labels Apr 7, 2010
    @ysjray
    Copy link
    Mannequin

    ysjray mannequin commented Apr 8, 2010

    The unittest of this patch.

    @amauryfa
    Copy link
    Member

    I also like the idea; 3 remarks though:

    • the patch introduces a new function that returns a PyObject*, but returns NULL when """the attribute is not found, and the caller should raise AttributeError""".
      This convention is not standard among the Python API and dangerous IMO.
      This part of the patch is not necessary. PyModule_GetAttr could just call PyObject_GenericGetAttr and override the current exception with a new message.

    • it's not necessary to expose the function PyModule_GetAttr. It could be renamed to something like module_getattr, and be a static function. Module writers are already used to PyObject_GetAttr to access the module items, this new function brings nothing new.

    • a minor nit: instead of
      module object 'mod_name' has no attribute 'xxx'
      I'd prefer
      module 'mod_name' has no attribute 'xxx'

    @ysjray
    Copy link
    Mannequin

    ysjray mannequin commented Jul 22, 2010

    Amaury:

    Thanks for reviewing! I quite appreciate your ideas. I was not quite familiar with python source code convention at that time. Here I worked out a new patch based on your ideas.

    Since py2.7 has released, this feature can only go to py3k. So my new patch is against py3k.

    Hoping somebody could review it. Thanks all!

    By the way, I feel if module object's getattr can be customized, other types, like the super object, also needs this.

    @durban
    Copy link
    Mannequin

    durban mannequin commented Jul 22, 2010

    According to PEP-7 [1], all declarations must be at the top of a block. So you probably should move the "char *mod_name_str" and "PyModuleObject *module" declarations to the beginning of the function's body.

    [1] http://www.python.org/dev/peps/pep-0007/

    @ysjray
    Copy link
    Mannequin

    ysjray mannequin commented Jul 23, 2010

    Thanks! durban.

    Here is the updated patch fixing such problem.

    @ethanfurman ethanfurman self-assigned this Oct 17, 2013
    @ethanfurman
    Copy link
    Member

    ysj.ray: Your patch looks good. I had to make some changes since we're now at 3.5.

    Before I can use your code, though, you need to sign the CLA [1].

    Thanks.

    [1] https://www.python.org/psf/contrib/contrib-form

    @ethanfurman
    Copy link
    Member

    Mostly new patch against 3.5

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 24, 2014

    New changeset d84a69b7ba72 by Ethan Furman in branch 'default':
    bpo-8297: module attribute lookup failures now include module name in error message.
    http://hg.python.org/cpython/rev/d84a69b7ba72

    @ethanfurman
    Copy link
    Member

    Yay, 'resolved' !

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

    No branches or pull requests

    5 participants