Issue13290
Created on 2011-10-29 07:04 by JBernardo, last changed 2012-11-13 06:42 by eric.snow.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| bltinmodule.c | JBernardo, 2011-10-29 07:04 | bltinmodule with `builtin_vars` function modified | ||
| issue13290.patch | maker, 2012-10-03 07:16 | review | ||
| Messages (18) | |||
|---|---|---|---|
| msg146598 - (view) | Author: João Bernardo (JBernardo) | Date: 2011-10-29 07:04 | |
I just realized the builtin function `vars` can't handle custom objects without the __dict__ attribute. It would be nice if it worked with objects that have __slots__ to make them look more like normal objects and to make debugging easier. I changed the source of "Python/bltinmodule.c" to accept this new case, but, as I'm fairly new to the C API, it may not be much good. I'm attaching the whole file to this Issue and the `builtin_vars` function was modified (lines 1859 to 1921) to work with: __slots__ = ('a', 'b', 'c') __slots__ = 'single_name' __slots__ = {'some': 'mapping'} The output is a dict (just like in the __dict__ case). #Example >>> class C: __slots__ = ['x', 'y'] >>> c = C() >>> c.x = 123 >>> vars(c) {'x': 123} |
|||
| msg147051 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2011-11-04 21:57 | |
New features only go in future versions: Given the doc: "With a module, class or class instance object as argument (or anything else that has a __dict__ attribute), return that attribute." So you are proposing a change in the definition of vars() input.
I agree that the proposal is a good idea as far as it goes. Vars() predates __slots__ (introduced in 2.2). In particular, adding or deleting a __slots__ binding to a user-defined class should not change the behavior of vars().
I am thinking that the doc should also be tweaked a bit. It implies that all class instances have a __dict__ attribute. Before 2.2, when 'class' meant a user-defined old style class, that was true. But with 'types' becoming new-style classes, it is not.
Perhaps the domain of vars() should be extended to all objects, returning {} if nothing more. I may bring that idea to python-ideas list.
You should attach diffs (context diffs, I believe) rather than files so we can see what you changed. The diff module can create such. I believe your changes come after the following:
d = PyObject_GetAttrString(v, "__dict__");
if (d == NULL) {
The statement '''slots = PyObject_GetAttrString(v, "__slots__")''' gets the attribute from the object itself. Like most special attributes other than __dict__, it should be gotten from the object's class (or superclasses). [I don't know the C version of doing this.] For example, suppose v is a class with a custom metaclass and type(v).__slots__ == ('__slots__', '__init__') whereas v.__slots__ == ('a', 'b') (and v.__init__ is not assigned). Then vars(v) should return {'__slots__': ('__slots__', '__init__')}.
The comment /* Find attributes out of __slots__ */ seems misleading. I believe it should be /* Convert string to iterable of 1 string */
New code for 3.3 should use the new unicode api from pep 393. Though I could be wrong, I believe PyUnicode_Check() has been replaced.
Your example "__slots__ = {'some': 'mapping'} is somewhat strange. It works, but only because iterating dicts iterates the keys, and the key is a string. As far as I know, the value is ignored and useless, so I hope no one does this. Perhaps your only point was to test a non-sequence iterable of strings.
A patch should include a patch to the appropriate Lib/test/testxxx.py file.
|
|||
| msg147057 - (view) | Author: João Bernardo (JBernardo) | Date: 2011-11-04 23:11 | |
Oh, sorry for the full file. Yes, I only changed after
d = PyObject_GetAttrString(v, "__dict__");
if (d == NULL) {
I was searching for uses of slots other than __slots__ = (a, b) and I saw a guy saying that dicts may have special meaning the future. So, something like
__slots__ = {'my_var': int}
could be an annotation (or hint for IDE's). In the present implementation, the value of each key is just ignored.
If slots is just "a_single_string" it should not be treated as an iterable, so I used PyUnicode_Check because I didn't knew a better method.
|
|||
| msg147063 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2011-11-05 00:22 | |
PyUnicode_Check may still be correct. I have not examined PEP393 in detail. |
|||
| msg147097 - (view) | Author: Éric Araujo (eric.araujo) * ![]() |
Date: 2011-11-05 16:20 | |
> You should attach diffs (context diffs, I believe) Nope, these are hard to read :) The universal diff format is the unified one. That’s also what Mercurial uses (see the devguide for more info on contributing). |
|||
| msg171863 - (view) | Author: Michele Orrù (maker) * | Date: 2012-10-03 07:16 | |
Patch + unittests + documentation attached. $ ./python -m test -R 3:2 test_builtin [1/1] test_builtin beginning 5 repetitions 12345 ..... 1 test OK. [158296 refs] |
|||
| msg171867 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2012-10-03 08:21 | |
I added some comments in Rietveld. ReST documentation should be updated too. vars() returns modifiable dict. The patch should be much harder if we want to preserve this behavior. Otherwise, the difference must be explicitly documented. |
|||
| msg171870 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2012-10-03 08:31 | |
There's another thing. __slots__ value is converted to a tuple and saved as ht_slots field in PyHeapTypeObject. You can use fast PyTuple_GET_ITEM() for item access. |
|||
| msg171878 - (view) | Author: Richard Oudkerk (sbt) * ![]() |
Date: 2012-10-03 10:59 | |
The patch does not seem to walk the mro to look for slots in base classes. Also, an instance with a __dict__ attribute may also have attributes stored in slots.
BTW, copyreg._slotnames(cls) properly calculates the slot names for cls and tries to cache them as cls.__slotnames__. Pickle does the equivalent of
try:
slotnames = cls.__slotnames__
except AttributeError:
slotnames = copyreg._slotnames(cls)
|
|||
| msg171893 - (view) | Author: Michele Orrù (maker) * | Date: 2012-10-03 16:07 | |
> The patch does not seem to walk the mro to look for slots in base > classes. Also, an instance with a __dict__ attribute may also have >attributes stored in slots. Well, what I am doing is more or less the equivalent of return object.__slots__ if hasattr(object, '__slots') else object.__dict__ and this is coherent with the updated documentation. The one you proposed is an alternative behavior; am I supposed to follow that one? > > BTW, copyreg._slotnames(cls) properly calculates the slot names for cls and tries to cache them as cls.__slotnames__. Pickle does the equivalent of > > try: > slotnames = cls.__slotnames__ > except AttributeError: > slotnames = copyreg._slotnames(cls) thanks! I'll take into account. -- ù |
|||
| msg171897 - (view) | Author: Richard Oudkerk (sbt) * ![]() |
Date: 2012-10-03 17:01 | |
> Well, what I am doing is more or less the equivalent of
>
> return object.__slots__ if hasattr(object, '__slots') else object.__dict__
>
> and this is coherent with the updated documentation. The one you
> proposed is an alternative behavior; am I supposed to follow that one?
Ignoring some slots but not others would be confusing. I would be inclined to just leave vars() alone. Maybe a Python implementation could be put in inspect.py instead.
A possible implementation (which can't be used to modify the object) might be:
import copyreg
def fullvars(obj):
cls = type(obj)
try:
slotnames = cls.__dict__['__slotnames__']
except (KeyError, AttributeError):
slotnames = copyreg._slotnames(cls)
try:
d = vars(obj).copy()
except TypeError:
d = {}
for name in slotnames:
try:
d[name] = getattr(obj, name)
except AttributeError:
pass
return d
class A:
__slots__ = 'x', 'y'
class B(A):
__slots__ = 'u', 'v'
class C(B):
pass
a = A()
a.x = 1
print(fullvars(a)) # {'x': 1}
b = B()
b.x = 2; b.u = 3
print(fullvars(b)) # {'u': 3, 'x': 2}
c = C()
c.y = 4; c.r = 5
print(fullvars(c)) # {'y': 4, 'r': 5}
BTW, I before should have written
try:
slotnames = cls.__dict__['__slotnames__']
except (KeyError, AttributeError):
slotnames = copyreg._slotnames(cls)
|
|||
| msg171899 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2012-10-03 17:13 | |
> The patch does not seem to walk the mro to look for slots in base classes. > Also, an instance with a __dict__ attribute may also have attributes > stored in slots. > > BTW, copyreg._slotnames(cls) properly calculates the slot names for cls and > tries to cache them as cls.__slotnames__. This is from Python side. Did ht_slots field of PyHeapTypeObject does not contain properly calculated slot names? |
|||
| msg171900 - (view) | Author: Richard Oudkerk (sbt) * ![]() |
Date: 2012-10-03 18:07 | |
> This is from Python side. Did ht_slots field of PyHeapTypeObject does not > contain properly calculated slot names? Looking at the code, it appears that ht_slots does *not* include inherited slots. |
|||
| msg171902 - (view) | Author: Terry J. Reedy (terry.reedy) * ![]() |
Date: 2012-10-03 19:02 | |
As I understand it, var(ob) is pretty much a synonym for ob.__dict__. Without knowing the internal details, I think vars should do with slots what it would do if slots were not used and normal dicts were used. In particular, if a class is changed by adding __slots__, vars(class) and vars(instances) should be as much the same as possible, at least for read-only uses. This allows iteration by keys, values, or items for introspection. That modifying the dict has no effect on the object is okay. It is the same for locals() inside a function. |
|||
| msg171904 - (view) | Author: Richard Oudkerk (sbt) * ![]() |
Date: 2012-10-03 19:23 | |
> That modifying the dict has no effect on the object is okay. I have written "vars(obj).update(...)" before. I don't think it would be okay to break that. |
|||
| msg171906 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2012-10-03 20:31 | |
I see a special warning in the documentation: Note: The returned dictionary should not be modified: the effects on the corresponding symbol table are undefined. [2] [2] In the current implementation, local variable bindings cannot normally be affected this way, but variables retrieved from other scopes (such as modules) can be. This may change. Therefore it's okay that modifying the returned dict has no effect on the object. |
|||
| msg171907 - (view) | Author: Richard Oudkerk (sbt) * ![]() |
Date: 2012-10-03 20:55 | |
A search of googlecode shows 42 examples of "vars(...).update" compared to 3000 for ".__dict__.update". I don't know if that is enough to worry about. http://code.google.com/codesearch#search/&q=vars\%28[A-Za-z0-9_]%2B\%29\.update%20lang:^python$&type=cs |
|||
| msg172171 - (view) | Author: Michele Orrù (maker) * | Date: 2012-10-06 06:49 | |
As a reference, linking the discussion on python-dev. http://mail.python.org/pipermail/python-dev/2012-October/122011.html |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2012-11-13 06:42:44 | eric.snow | set | nosy:
+ eric.snow |
| 2012-10-06 06:49:04 | maker | set | messages: + msg172171 |
| 2012-10-03 20:55:38 | sbt | set | messages: + msg171907 |
| 2012-10-03 20:31:36 | serhiy.storchaka | set | messages: + msg171906 |
| 2012-10-03 19:23:22 | sbt | set | messages: + msg171904 |
| 2012-10-03 19:02:16 | terry.reedy | set | messages: + msg171902 |
| 2012-10-03 18:07:26 | sbt | set | messages: + msg171900 |
| 2012-10-03 17:13:18 | serhiy.storchaka | set | messages: + msg171899 |
| 2012-10-03 17:01:47 | sbt | set | messages: + msg171897 |
| 2012-10-03 16:07:13 | maker | set | messages: + msg171893 |
| 2012-10-03 10:59:37 | sbt | set | nosy:
+ sbt messages: + msg171878 |
| 2012-10-03 08:31:17 | serhiy.storchaka | set | messages: + msg171870 |
| 2012-10-03 08:21:02 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages: + msg171867 versions: + Python 3.4, - Python 3.3 |
| 2012-10-03 07:16:10 | maker | set | files:
+ issue13290.patch nosy: + maker messages: + msg171863 keywords: + patch |
| 2011-11-05 16:20:14 | eric.araujo | set | nosy:
+ rhettinger |
| 2011-11-05 16:20:04 | eric.araujo | set | nosy:
+ eric.araujo messages: + msg147097 |
| 2011-11-05 00:22:01 | terry.reedy | set | messages: + msg147063 |
| 2011-11-04 23:11:17 | JBernardo | set | messages: + msg147057 |
| 2011-11-04 21:57:56 | terry.reedy | set | versions:
- Python 3.2 nosy: + terry.reedy messages: + msg147051 stage: needs patch |
| 2011-10-29 07:04:10 | JBernardo | create | |
