This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: importlib.metadata.DeprecatedList appears to be missing __slots__
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.11, Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ariebovenberg, jaraco, miss-islington
Priority: normal Keywords: patch

Created on 2022-01-03 18:39 by ariebovenberg, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 30452 merged ariebovenberg, 2022-01-07 12:26
PR 31269 merged miss-islington, 2022-02-11 00:56
Messages (7)
msg409604 - (view) Author: Arie Bovenberg (ariebovenberg) * Date: 2022-01-03 18:39
(as instructed in bpo-46244, I've created this ticket)

The subclass `EntryPoints` defines __slots__, but its base `DeprecatedList` does not.

This appears to be a mistake.

If so, I'd like to create a PR to fix it.
msg409644 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2022-01-04 00:13
Perhaps it is a mistake. The `__slots__` were added as a (possible premature) optimization in [this conversation](https://github.com/python/importlib_metadata/pull/278/files#r565475347).

It's not obvious to me what the danger is in defining __slots__ in a child class but not in a parent. Can you point me to resources or examples that help me understand the concern or missed expectation? The primary motivation behind defining slots is to help ensure immutability.  Does the lack of slots violate that expectation?

Convince me of the value of fixing this concern and please feel free to proceed with a PR.
msg409652 - (view) Author: Arie Bovenberg (ariebovenberg) * Date: 2022-01-04 07:57
@jaraco thanks for your quick response.

In short: __slots__ allows class layout to be optimized by replacing the class __dict__ with specific descriptors. This results in a class where only specific attributes can be get/set.

However, you only really get the savings from __slots__ if all base classes also implement it.

An example:

    from pympler.asizeof import asizeof  # checks complete size of objects in memory

    class EmptyNoSlots: pass

    class EmptyWithSlots: __slots__ = ()

    class NoSlots:
        def __init__(self, a, b): self.a, self.b = a, b

    class WithSlots:
        __slots__ = ("a", "b")
        def __init__(self, a, b): self.a, self.b = a, b

    print(asizeof(EmptyNoSlots()))    # 152
    print(asizeof(EmptyWithSlots()))  # 32
    print(asizeof(NoSlots(1, 2)))     # 328
    print(asizeof(WithSlots(1, 2)))   # 112

    # Let's look at inheritance:

    class WithSlotsAndProperBaseClass(EmptyWithSlots):
        __slots__ = ("a", "b")
        def __init__(self, a, b): self.a, self.b = a, b

    class NoSlotsAtAll(EmptyNoSlots):
        def __init__(self, a, b): self.a, self.b = a, b

    class WithSlotsAndBadBaseClass(EmptyNoSlots):
        __slots__ = ("a", "b")
        def __init__(self, a, b): self.a, self.b = a, b

    print(asizeof(WithSlotsAndProperBaseClass(1, 2)))  # 112
    print(asizeof(NoSlotsAtAll(1, 2)))                 # 328
    print(asizeof(WithSlotsAndBadBaseClass(1, 2)))     # 232 -- oh no!

In importlib:

                       list   <- has slots (builtin)
       DeprecatedList(list)   <- no __slots__ defined
EntryPoints(DeprecatedList)   <- defines __slots__

Besides the lost memory savings, because `DeprecatedList` has no slots, you can still do:

    EntryPoints().foo = 6  # setting a random attribute, not the intention.

Further reading: https://stackoverflow.com/a/28059785
msg409680 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2022-01-04 14:09
Today I learned something. Thanks Arie. Yes, I agree that's a mistake. Perhaps the test suite should also have a test to capture the missed expectation (that __dict__ should not be present or setting attributes on EntryPoints instances should fail).
msg413031 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2022-02-11 00:45
I'm pretty sure both EntryPoints and DeprecatedList were introduced in Python 3.10, so 3.9 and 3.8 aren't relevant.
msg413032 - (view) Author: miss-islington (miss-islington) Date: 2022-02-11 00:56
New changeset dd76b3f7d332dd6eced5cbc2ad2adfc397700b3d by Arie Bovenberg in branch 'main':
bpo-46246: add missing __slots__ to importlib.metadata.DeprecatedList (GH-30452)
https://github.com/python/cpython/commit/dd76b3f7d332dd6eced5cbc2ad2adfc397700b3d
msg413034 - (view) Author: miss-islington (miss-islington) Date: 2022-02-11 01:18
New changeset 1124ab6d1d15dc5058e03b63fd1d95e6f1009cc3 by Miss Islington (bot) in branch '3.10':
bpo-46246: add missing __slots__ to importlib.metadata.DeprecatedList (GH-30452)
https://github.com/python/cpython/commit/1124ab6d1d15dc5058e03b63fd1d95e6f1009cc3
History
Date User Action Args
2022-04-11 14:59:54adminsetgithub: 90404
2022-03-14 07:35:48jaracosetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2022-02-11 01:18:31miss-islingtonsetmessages: + msg413034
2022-02-11 00:56:38miss-islingtonsetpull_requests: + pull_request29433
2022-02-11 00:56:25miss-islingtonsetnosy: + miss-islington
messages: + msg413032
2022-02-11 00:45:19jaracosetmessages: + msg413031
versions: - Python 3.8, Python 3.9
2022-01-07 12:26:48ariebovenbergsetkeywords: + patch
stage: patch review
pull_requests: + pull_request28656
2022-01-04 14:09:40jaracosetmessages: + msg409680
2022-01-04 07:57:52ariebovenbergsetmessages: + msg409652
2022-01-04 00:13:08jaracosetmessages: + msg409644
2022-01-03 21:39:28AlexWaygoodsetnosy: + jaraco
2022-01-03 18:39:08ariebovenbergcreate