getattr function w/ default

[glchapman]

BPO	420304
Nosy	@mwhudson, @malemburg, @tim-one, @doerwalter, @chmarr

^{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 2001-07-30.22:41:43.000>
created_at = <Date 2001-05-01.00:14:28.000>
labels = ['interpreter-core']
title = 'getattr function w/ default'
updated_at = <Date 2001-07-30.22:41:43.000>
user = 'https://bugs.python.org/glchapman'

bugs.python.org fields:

activity = <Date 2001-07-30.22:41:43.000>
actor = 'jhylton'
assignee = 'jhylton'
closed = True
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2001-05-01.00:14:28.000>
creator = 'glchapman'
dependencies = []
files = []
hgrepos = []
issue_num = 420304
keywords = []
message_count = 9.0
messages = ['4566', '4567', '4568', '4569', '4570', '4571', '4572', '4573', '4574']
nosy_count = 7.0
nosy_names = ['mwh', 'lemburg', 'tim.peters', 'jhylton', 'doerwalter', 'chriscog', 'glchapman']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue420304'
versions = []

[glchapman]

In Python 2.1, when calling the builtin getattr
function and supplying the (optional) default
argument, this default is returned on any exception
raised during PyObject_GetAttr, even if the exception
is not an AttributeError. I did not expect this
behavior after reading the getattr documentation. Is
this the intended behavior? (If so, I think the
documentation should be made clearer on this point.)

Example (providing a non-string as the attribute name):

>>> class Test:
... 	pass
... 
>>> t = Test()
>>> getattr(t, 1, "hello")
'hello'
>>>

[chmarr]

Logged In: YES
user_id=67116

Looks like an attribute error to me. consider this:

>>> getattr (t,fred,"thingy")
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
NameError: There is no variable named 'fred'

Ie, in your own example, it couldn't find an attribute '1'
in the class, even if its an integer attribute (which is
only obtainable using getattr/setattr). That's exactly what
AttributeError is

[mwhudson]

Logged In: YES
user_id=6656

But consider this:

>>> getattr({},1)
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
TypeError: attribute name must be string
>>> getattr({},1,"e")
'e'

or

>>> getattr(None,u"\343","e")
'e'
>>> getattr(None,u"\343")
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
UnicodeError: ASCII encoding error: ordinal not in range(128)

Here's a possible patch:

Index: bltinmodule.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Python/bltinmodule.c,v
retrieving revision 2.200
diff -c -r2.200 bltinmodule.c
*** bltinmodule.c 2001/05/02 07:39:38 2.200
--- bltinmodule.c 2001/05/04 11:09:35

---

*** 845,850 ****
--- 845,854 ----
The globals and locals are dictionaries, defaulting to the
current\n\
globals and locals. If only globals is given, locals
defaults to it.";

+ /* Internal API needed by builtin_getattr(): */
+ extern
+ PyObject *_PyUnicode_AsDefaultEncodedString(PyObject *unicode,
+ const char *errors);

static PyObject *
builtin_getattr(PyObject *self, PyObject *args)

---

*** 854,859 ****
--- 858,874 ----

  
        if (!PyArg_ParseTuple(args, "OO|O:getattr", &v,
&name, &dflt))
                return NULL;
+       if (PyUnicode_Check(name)) {
+               name =
_PyUnicode_AsDefaultEncodedString(name, NULL);
+               if (name == NULL)
+                       return NULL;
+       } 
+       else if (!PyString_Check(name)) {
+               PyErr_Format(PyExc_TypeError,
+            "getattr: attribute name must be string, not
\"%.500s\"",
+                            name->ob_type->tp_name);
+               return NULL;
+       }
        result = PyObject_GetAttr(v, name);
        if (result == NULL && dflt != NULL) {
                PyErr_Clear();

... though by the time you've done all that, there's not
much point in calling PyObject_GetAttr at all...

OTOH, this changes behaviour, so maybe one should just
clarify this in the docs.

[jhylton]

Logged In: YES
user_id=31392

This looks like a bug to me, too. I think a non-string attr
argument
to getattr() should raise TypeError.

[doerwalter]

Logged In: YES
user_id=89016

But unicode attr arguments should be allowed, because
someday in the far future Python might be unicode based.

[mwhudson]

Logged In: YES
user_id=6656

Well, the patch I posted below allows unicode (though
admittedly only for strings that can be encoded using the
default encoding).

Have you read the code to object.c:PyObject_GetAttr? All my
code does is hoist some of the error checking out of there
into getattr to better decide whether to return the provided
default value.

[malemburg]

Logged In: YES
user_id=38388

I guess the right thing to do here, would be to only mask
AttributeErrors in the getattr() builtin.

[tim-one]

Logged In: YES
user_id=31435

Oh no: getattr hooks can raise any exception whatsoever,
and existing code relies on getattr() treating any
exception whatsoever as meaning "no attribute of that
name". For example, IndexError and KeyError are often
raised naturally by getattr hooks that fail to resolve a
name via tree or dict structures.

The specific complaint here is a different story: the docs
say the attribute argument must be a string (Unicode,
whatever -- "a string" <wink>), and I agree that calling
getattr() with something other than a string argument
should raise a TypeError instead of pretending it's a
*possible* attribute name and that there simply isn't any
attribute with that name (it's not a name at all, so the
call doesn't make sense).

Jeremy, I see this is assigned to you now. There are
enough conflicting opinions here (at least Michael's and
mine seem to coincide) that perhaps it's time to dump it on
Guido for Pronouncement.

[jhylton]

Logged In: YES
user_id=31392

Don't think there's any actual dissent here. You, Michael,
and I all seem to agree. I'll commit Michael's suggestion
and live with the duplicated code.

Fixed in rev 2.220 of bltinmodule.c