classification
Title: AttributeError message text should include module name
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ethan.furman Nosy List: ajaksu2, amaury.forgeotdarc, chris.jerdonek, daniel.urban, ethan.furman, ezio.melotti, python-dev, r.david.murray, ysj.ray
Priority: low Keywords: patch

Created on 2010-04-03 05:30 by chris.jerdonek, last changed 2014-04-25 00:14 by ethan.furman. This issue is now closed.

Files
File name Uploaded Description Edit
issue_8297.diff ysj.ray, 2010-04-06 13:14 Patch for trunk
issue_8297_test.py ysj.ray, 2010-04-08 07:46 Patch for trunk
issue_8297.diff ysj.ray, 2010-07-22 12:45 patch against py3k
issue8297.stoneleaf.01.patch ethan.furman, 2014-03-27 05:15 review
Messages (15)
msg102224 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2010-04-03 05:30
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'
msg102229 - (view) Author: ysj.ray (ysj.ray) Date: 2010-04-03 08:34
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'.
msg102232 - (view) Author: ysj.ray (ysj.ray) Date: 2010-04-03 09:17
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?
msg102463 - (view) Author: ysj.ray (ysj.ray) Date: 2010-04-06 13:14
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.
msg102514 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2010-04-07 02:54
Great -- thanks a lot for taking a stab at this!
msg102549 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-04-07 17:26
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.
msg102598 - (view) Author: ysj.ray (ysj.ray) Date: 2010-04-08 07:46
The unittest of this patch.
msg111153 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-07-22 07:01
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'
msg111171 - (view) Author: ysj.ray (ysj.ray) Date: 2010-07-22 12:45
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.
msg111200 - (view) Author: Daniel Urban (daniel.urban) * Date: 2010-07-22 17:45
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/
msg111232 - (view) Author: ysj.ray (ysj.ray) Date: 2010-07-23 02:19
Thanks! durban.

Here is the updated patch fixing such problem.
msg214434 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-03-22 03:13
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
msg214936 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-03-27 05:15
Mostly new patch against 3.5
msg217139 - (view) Author: Roundup Robot (python-dev) Date: 2014-04-24 21:48
New changeset d84a69b7ba72 by Ethan Furman in branch 'default':
Issue8297: module attribute lookup failures now include module name in error message.
http://hg.python.org/cpython/rev/d84a69b7ba72
msg217142 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2014-04-25 00:14
Yay, 'resolved' !
History
Date User Action Args
2014-04-25 00:14:09ethan.furmansetmessages: + msg217142
stage: patch review -> resolved
2014-04-24 23:48:55benjamin.petersonsetstatus: open -> closed
resolution: fixed
2014-04-24 21:48:00python-devsetnosy: + python-dev
messages: + msg217139
2014-03-27 05:15:17ethan.furmansetfiles: + issue8297.stoneleaf.01.patch

messages: + msg214936
2014-03-22 03:13:57ethan.furmansetmessages: + msg214434
2014-03-21 23:49:26ethan.furmansetversions: + Python 3.5, - Python 3.4
2013-10-17 04:24:34ethan.furmansetassignee: ethan.furman
versions: + Python 3.4, - Python 2.7, Python 3.2
2013-05-18 04:44:39ethan.furmansetnosy: + ethan.furman
2010-07-23 02:19:23ysj.raysetmessages: + msg111232
2010-07-22 17:45:02daniel.urbansetnosy: + daniel.urban
messages: + msg111200
2010-07-22 12:45:14ysj.raysetfiles: + issue_8297.diff

messages: + msg111171
2010-07-22 07:01:25amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg111153
2010-04-13 21:20:54ajaksu2setnosy: + ajaksu2

stage: test needed -> patch review
2010-04-08 07:46:08ysj.raysetfiles: + issue_8297_test.py

messages: + msg102598
2010-04-07 17:26:13r.david.murraysetversions: + Python 3.2, - Python 2.6
nosy: + r.david.murray

messages: + msg102549

components: + Interpreter Core, - Library (Lib)
stage: needs patch -> test needed
2010-04-07 05:15:30chris.jerdoneksetversions: + Python 2.6
2010-04-07 02:54:32chris.jerdoneksetmessages: + msg102514
2010-04-06 13:14:43ysj.raysetfiles: + issue_8297.diff
keywords: + patch
messages: + msg102463
2010-04-03 22:21:42ezio.melottisetpriority: low
nosy: + ezio.melotti

stage: needs patch
2010-04-03 09:17:19ysj.raysetmessages: + msg102232
2010-04-03 08:34:14ysj.raysetnosy: + ysj.ray
messages: + msg102229
2010-04-03 05:30:05chris.jerdonekcreate