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

replacing obj.__dict__ with a subclass of dict #43272

Closed
gangesmaster mannequin opened this issue Apr 24, 2006 · 10 comments
Closed

replacing obj.__dict__ with a subclass of dict #43272

gangesmaster mannequin opened this issue Apr 24, 2006 · 10 comments
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@gangesmaster
Copy link
Mannequin

gangesmaster mannequin commented Apr 24, 2006

BPO 1475692
Nosy @rhettinger, @devdanzin, @bitdancer, @Bluehorn, @ericsnowcurrently, @serhiy-storchaka, @kushaldas, @MojoVampire, @iritkatriel

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 2020-11-07.20:13:38.119>
created_at = <Date 2006-04-24.17:45:28.000>
labels = ['interpreter-core', 'type-bug', '3.8', '3.9', '3.10']
title = 'replacing obj.__dict__ with a subclass of dict'
updated_at = <Date 2020-11-07.21:56:36.404>
user = 'https://bugs.python.org/gangesmaster'

bugs.python.org fields:

activity = <Date 2020-11-07.21:56:36.404>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2020-11-07.20:13:38.119>
closer = 'rhettinger'
components = ['Interpreter Core']
creation = <Date 2006-04-24.17:45:28.000>
creator = 'gangesmaster'
dependencies = []
files = []
hgrepos = []
issue_num = 1475692
keywords = []
message_count = 10.0
messages = ['60907', '83910', '92419', '188082', '259213', '259215', '259257', '380486', '380523', '380525']
nosy_count = 11.0
nosy_names = ['rhettinger', 'gangesmaster', 'ajaksu2', 'r.david.murray', 'matthieu.labbe', 'torsten', 'eric.snow', 'serhiy.storchaka', 'kushal.das', 'josh.r', 'iritkatriel']
pr_nums = []
priority = 'low'
resolution = 'wont fix'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue1475692'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

@gangesmaster
Copy link
Mannequin Author

gangesmaster mannequin commented Apr 24, 2006

>>> class mydict(dict):
...     def __getitem__(self, key):
...         return 17
...
>>> class blah(object):
...     def __init__(self):
...         self.__dict__ = mydict()
...
>>> b = blah()
>>> print b.x
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
AttributeError: 'blah' object has no attribute 'x'

python doesn't call the overriden version of __getitem__.

i've done several more tests, and the cause to this
problem, afaik, is that the code assumes __dict__ is an
instance of dict, so it directly uses PyDict_GetItem
(or whatever it's called), thus skipping the overriden
method.

python should either disable setting __dict__ to
anything that is not a real dict (type(x) == dict
instead of isinstance(x, dict)), or be willing to call
overriden methods.

@gangesmaster gangesmaster mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Apr 24, 2006
@devdanzin
Copy link
Mannequin

devdanzin mannequin commented Mar 21, 2009

Confirmed, is this a valid issue?

@devdanzin devdanzin mannequin added the type-feature A feature request or enhancement label Mar 21, 2009
@matthieulabbe matthieulabbe mannequin added type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels Sep 8, 2009
@bitdancer
Copy link
Member

2.5 and 2.4 are in security-fix-only mode, so we don't set them in
versions since bugs won't get fixed there.

I don't think overridden methods should be called, since that would slow
down attribute lookup, which is already a bottleneck in Python. Whether
there should be an error message is a more complicated question, and I'd
have to look at how this is implemented to even have an opinion on that.

@kushaldas
Copy link
Member

In Objects/typeobject.c we have subtype_setdict function, in which at line 1830 we have PyDict_Check() macro call, which checks if it is a subclass of dict or not.

The definition is in Include/dictobject.h

#define PyDict_Check(op) \
                 PyType_FastSubclass(Py_TYPE(op), Py_TPFLAGS_DICT_SUBCLASS)

We can stop assigning anything other than dict in typesobject.c but that will break other things like http://bugs.python.org/issue1475692

@Bluehorn
Copy link
Mannequin

Bluehorn mannequin commented Jan 29, 2016

I just bumped into this issue because I was shown by a colleague that my implementation of immutable objects (by replacing __dict__ with an ImmutableDict that inherits from dict and blocks write accesses) is ineffective - ouch!

I'd expect that Python either rejects subclasses of dict for obj.__dict__ or actually implements accessing correctly. Especially since the enum module created the impression for me that replacing __dict__ is a viable approach (enum.py uses __prepare__ in the meta class to provide a different dict class for enum types, just found https://www.python.org/dev/peps/pep-3115/).

Interestingly the PEP-3115 example code notes the following:

# Note that we replace the classdict with a regular
# dict before passing it to the superclass, so that we
# don't continue to record member names after the class
# has been created.

@serhiy-storchaka
Copy link
Member

Python uses concrete class API (PyDict_GetItem and like) for resolving attributes. Using general mapping API would slow down attribute lookup in common case. This is performance critical part of Python and we should be very careful changing it.

On the other side, you still can have a benefit from using dict subclasses as __dict__. OrderedDict allows you to iterate dict in predefined order, and defaultdict allows you to create attributes with default values on demand (but __getattr__ is more universal method).

See also bpo-10977.

@MojoVampire
Copy link
Mannequin

MojoVampire mannequin commented Jan 30, 2016

Just FYI, if you're only trying to make immutable objects, that's what subclassing tuple with properties and __slots__ = () is for (collections.namedtuple does exactly this, building the Python declaration as a string and then eval-ing it to produce a tuple subclass with named property accessors). The only negative is that it still acts like a sequence, but usually that's not a big problem.

@iritkatriel
Copy link
Member

Calling the overridden __getitem__ is rejected due to performance.

Forbidding dict subclasses is rejected because subclasses like ordereddict and defaultdict can be useful.

I think the only remaining possibilities are to do nothing or to raise an error when __dict__ is set to a dict subclass that overrides __getitem__ (that should be ok to do, because that __getitem__ is not going to be called).

@iritkatriel iritkatriel added 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes labels Nov 7, 2020
@rhettinger
Copy link
Contributor

I'm going to mark this as rejected. After 14 years, no clean and performant solution has emerged, yet the world of Python seems to be fine with the status quo. This issue doesn't seem to be getting in the way of people doing their job.

@serhiy-storchaka
Copy link
Member

There is no longer need to use OrderedDict as __dict__, but ctypes types have tp_dict which are instances of dict subclass (StgDict). Disabling setting it to anything that is not an exact dict would break ctypes.

@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
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

5 participants