AttributeError message text should include module name

[cjerdonek]

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]

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'

[ysjray]

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]

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]

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]

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

[bitdancer]

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.

[ysjray]

The unittest of this patch.

[amauryfa]

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]

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]

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]

Thanks! durban.

Here is the updated patch fixing such problem.