From 3a1dd2164f8bdd2d15ef1d9cc2ce74788e11c1a2 Mon Sep 17 00:00:00 2001 From: nstarman Date: Thu, 6 Aug 2026 12:22:09 -0400 Subject: [PATCH 1/5] =?UTF-8?q?=F0=9F=90=9B=20fix:=20stop=20members=20with?= =?UTF-8?q?=20equal=20values=20collapsing=20into=20aliases?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `enum.Enum` folds any member whose value compares equal to an earlier one into an alias of that member -- silently, keeping the first name. The values here are versions, which makes that the common case rather than a corner: ```python class OptDeps(OptionalDependencyEnum): NOT_A_PACKAGE = auto() ALSO_NOT_A_PACKAGE = auto() OptDeps.ALSO_NOT_A_PACKAGE.name # 'NOT_A_PACKAGE' <- wrong member ``` Two failure modes, both silent: - **Any two uninstalled dependencies.** They share the one `InstalledState.NOT_INSTALLED` sentinel, so the second collapses into the first. Every enum with more than one missing dependency is affected. - **Distributions released together.** They share a version number, so members of a co-released family collapse. `unxt` hit exactly this: `unxts.api`, `unxts.hypothesis` and `unxts.parametric` are all 2.0.0, so it cannot put them in an `OptionalDependencyEnum` at all and detects them with a hand-rolled `find_spec` helper instead. In both cases the surviving member answers `.installed`/`.version` for the wrong package, which is the one question this library exists to answer. The fix keys `_value_` on a per-member `_MemberKey` wrapper, distinct by identity, so no two members can compare equal. It has to happen in `__new__`: from 3.11 on, `enum` snapshots `_value_` for the duplicate scan *before* calling `__init__`, so re-keying there is too late (3.10 tolerates it, 3.11+ does not -- worth knowing if this is ever revisited). The public surface is unchanged: - `value` is a `DynamicClassAttribute` unwrapping the key, so it is still the `Version` or `NOT_INSTALLED`, and `installed`/`version`/the comparators read through it untouched. - `_MemberKey.__repr__` delegates, so members still show as `>`. - `_missing_` restores `OptDeps(some_version)` by-value lookup, with the ambiguity it always had: first declared member wins. - `__reduce_ex__` pickles by name, which `_value_` can no longer do -- and which is exact, where by-value never was. Tested on 3.10-3.14: 49 passed on each. Unrelated, noticed while writing the tests: the README's truthiness section (`if not OptDeps.THIS_IS_NOT_INSTALLED:`) does not hold -- `Enum` members are always truthy, and no `__bool__` is defined. README code blocks are not collected by Sybil (`patterns=["*.rst", "*.py"]`), so nothing caught it. Left alone here; happy to follow up with either a `__bool__` or a README fix, whichever you prefer. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 --- README.md | 20 +++++ src/optional_dependencies/_core.py | 97 ++++++++++++++++++++++- tests/test_no_aliasing.py | 120 +++++++++++++++++++++++++++++ 3 files changed, 235 insertions(+), 2 deletions(-) create mode 100644 tests/test_no_aliasing.py diff --git a/README.md b/README.md index 70b7451..c6d9c71 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,26 @@ then then [`packaging.version.parse`][Version-link]. If the package cannot be found then it is considered `InstalledState.NOT_INSTALLED` +Every member stays its own member, whatever its value. This matters because +those values collide readily: any two dependencies that are both missing share +the `NOT_INSTALLED` sentinel, and distributions released together share a +version number. A plain [`enum.Enum`][Enum-link] folds equal-valued members into +aliases of whichever was declared first, which would make +`OptDeps.SECOND.installed` report the _first_ package's state. + +```python +class OptDeps(OptionalDependencyEnum): + NOT_A_PACKAGE = auto() + ALSO_NOT_A_PACKAGE = auto() + + +OptDeps.ALSO_NOT_A_PACKAGE.name +# 'ALSO_NOT_A_PACKAGE' + +OptDeps.NOT_A_PACKAGE is OptDeps.ALSO_NOT_A_PACKAGE +# False +``` + `InstalledState.NOT_INSTALLED` is an [`enum.Enum`][Enum-link] member that has a truthy value of `False`. This can be useful for boolean checks, as [`packaging.Version`][Version-link] always has a truthy value of `True`. diff --git a/src/optional_dependencies/_core.py b/src/optional_dependencies/_core.py index 85f2786..f11438c 100644 --- a/src/optional_dependencies/_core.py +++ b/src/optional_dependencies/_core.py @@ -6,14 +6,18 @@ from collections.abc import Callable from dataclasses import dataclass from enum import Enum -from types import MethodType -from typing import Literal, cast +from types import DynamicClassAttribute, MethodType +from typing import Literal, TypeVar, cast, final from packaging.utils import canonicalize_name from packaging.version import Version from .utils import InstalledState, get_version +#: Stands in for `typing.Self`, which is 3.11+; this package supports 3.10, and +#: `typing_extensions` is not a dependency. +_EnumT = TypeVar("_EnumT", bound="OptionalDependencyEnum") + @dataclass(frozen=True) class Comparator: @@ -73,9 +77,98 @@ def __call__(self, enum: "OptionalDependencyEnum", other: object) -> bool: # =================================================================== +@final +class _MemberKey: + """The internal ``_value_`` of an `OptionalDependencyEnum` member. + + `enum.Enum` folds any member whose ``_value_`` compares equal to an earlier + one into an alias of that member -- silently, keeping the first name. A + version cannot serve as that key: every uninstalled dependency carries the + same `InstalledState.NOT_INSTALLED` sentinel, and distributions released + together share a version number, so a second uninstalled member, or a second + member of a co-released family, used to vanish into the first and report the + wrong package's state. + + Wrapping the resolved version in one of these gives every member a key that + is distinct by identity, so no two of them can ever compare equal. The + wrapper is internal: `OptionalDependencyEnum.value` unwraps it, and `__repr__` + delegates so members still show as ``>``. + """ + + __slots__ = ("resolved",) + + def __init__( + self, resolved: Version | Literal[InstalledState.NOT_INSTALLED], / + ) -> None: + self.resolved = resolved + + def __repr__(self) -> str: + return repr(self.resolved) + + class OptionalDependencyEnum(Enum): """An enumeration of optional dependencies.""" + _value_: _MemberKey + + # PYI019 wants `typing.Self` here; see `_EnumT` for why it cannot be used. + def __new__( # noqa: PYI019 + cls: type[_EnumT], value: Version | Literal[InstalledState.NOT_INSTALLED] + ) -> _EnumT: + """Give the member an alias-proof ``_value_``. + + See `_MemberKey` for why the resolved version cannot be that key. + + This has to happen in ``__new__`` rather than ``__init__``: from Python + 3.11 on, `enum` snapshots ``_value_`` for the duplicate scan *before* + calling ``__init__``, so a reassignment there comes too late. Every + supported version (3.10+) honours a ``_value_`` set in ``__new__``. + """ + obj = object.__new__(cls) + obj._value_ = _MemberKey(value) + return obj + + @classmethod + def _missing_(cls, value: object) -> "OptionalDependencyEnum | None": + """Look a member up by its version, as ``Enum(value)`` used to. + + Members are keyed internally on `_MemberKey`, so the by-value map no + longer holds bare versions. This restores the lookup, with the ambiguity + it always had: where several members share a version, the first one + declared wins. + """ + return next((m for m in cls if m._value_.resolved == value), None) + + def __reduce_ex__(self, proto: object) -> tuple[object, ...]: + """Pickle by name. + + `enum.Enum` pickles by ``_value_``, which is now an identity-keyed + wrapper that would not survive the round trip. The name is the exact + key besides: versions do not distinguish members, which is the whole + problem being fixed here. + """ + return getattr, (self.__class__, self._name_) + + @DynamicClassAttribute + def value(self) -> Version | Literal[InstalledState.NOT_INSTALLED]: + """The version of the optional dependency, or `NOT_INSTALLED`. + + Examples + -------- + >>> from enum import auto + >>> class OptDeps(OptionalDependencyEnum): + ... PACKAGING = auto() + ... NOTINSTALLED = auto() + + >>> OptDeps.PACKAGING.value + + + >>> OptDeps.NOTINSTALLED.value + + + """ + return self._value_.resolved + @staticmethod def _generate_next_value_( name: str, diff --git a/tests/test_no_aliasing.py b/tests/test_no_aliasing.py new file mode 100644 index 0000000..ae42919 --- /dev/null +++ b/tests/test_no_aliasing.py @@ -0,0 +1,120 @@ +"""Every member stays its own member, whatever its value. + +`enum.Enum` folds members whose values compare equal into a single member, +keeping the first name and aliasing the rest. The values here are versions, so +collisions are the norm rather than the exception: every uninstalled dependency +carries the same `NOT_INSTALLED` sentinel, and co-released distributions share a +version number. +""" + +import pickle + +import pytest +from packaging.version import Version + +from optional_dependencies import OptionalDependencyEnum, auto +from optional_dependencies.utils import NOT_INSTALLED, get_version, is_installed + + +class OptDeps(OptionalDependencyEnum): + PACKAGING = auto() # runtime dependency + PYTEST = auto() # test dependency + NOTINSTALLED = auto() # not installed + ALSONOTINSTALLED = auto() # not installed either + # Low-level API: an explicitly assigned value, colliding with PACKAGING. + PACKAGING_AGAIN = get_version("packaging") + + +def test_all_members_are_distinct() -> None: + """Iteration yields every member, none folded into an alias.""" + assert [m.name for m in OptDeps] == list(OptDeps.__members__) + assert len(set(OptDeps)) == 5 + + +@pytest.mark.parametrize("name", list(OptDeps.__members__)) +def test_member_keeps_its_own_name(name: str) -> None: + """Attribute lookup returns the member it was asked for.""" + assert OptDeps[name].name == name + assert getattr(OptDeps, name).name == name + + +def test_two_uninstalled_members_do_not_collapse() -> None: + """Both share the `NOT_INSTALLED` sentinel; neither becomes the other.""" + assert OptDeps.NOTINSTALLED is not OptDeps.ALSONOTINSTALLED + assert not OptDeps.NOTINSTALLED.installed + assert not OptDeps.ALSONOTINSTALLED.installed + + +def test_two_members_sharing_a_version_do_not_collapse() -> None: + """An explicit value equal to another member's is still its own member.""" + assert OptDeps.PACKAGING is not OptDeps.PACKAGING_AGAIN + assert OptDeps.PACKAGING.version == OptDeps.PACKAGING_AGAIN.version + + +def test_value_is_still_the_version() -> None: + """`value` keeps its documented meaning: a `Version` or `NOT_INSTALLED`.""" + assert isinstance(OptDeps.PACKAGING.value, Version) + assert OptDeps.PACKAGING.value == OptDeps.PACKAGING.version + assert OptDeps.NOTINSTALLED.value is NOT_INSTALLED + + +def test_members_are_hashable_and_usable_as_keys() -> None: + """Distinct members occupy distinct slots in a dict/set.""" + mapping = {m: m.name for m in OptDeps} + assert len(mapping) == 5 + assert mapping[OptDeps.ALSONOTINSTALLED] == "ALSONOTINSTALLED" + + +@pytest.mark.parametrize("name", list(OptDeps.__members__)) +def test_roundtrips_through_pickle(name: str) -> None: + """Pickling resolves back to the same member, not to an alias.""" + member = OptDeps[name] + assert pickle.loads(pickle.dumps(member)) is member # noqa: S301 + + +def test_lookup_by_value_still_works() -> None: + """`Enum(value)` keeps resolving through the version, as before. + + It cannot distinguish members that share one -- that ambiguity is inherent + to looking a member up by a non-unique key -- but it must still find *a* + member rather than raise. + """ + assert OptDeps(get_version("packaging")) in { + OptDeps.PACKAGING, + OptDeps.PACKAGING_AGAIN, + } + assert OptDeps(NOT_INSTALLED) in {OptDeps.NOTINSTALLED, OptDeps.ALSONOTINSTALLED} + + +def test_lookup_by_unknown_value_still_raises() -> None: + """A version no member carries is still a `ValueError`.""" + with pytest.raises(ValueError, match="is not a valid OptDeps"): + OptDeps(Version("0.0.0.dev0")) + + +def test_lookup_by_name_still_works() -> None: + """`Enum[name]` is exact, and now the only exact lookup.""" + assert OptDeps["PACKAGING_AGAIN"] is OptDeps.PACKAGING_AGAIN + assert OptDeps["ALSONOTINSTALLED"] is OptDeps.ALSONOTINSTALLED + + +def test_class_level_value_access_is_rejected() -> None: + """`value` stays a per-member attribute, as on a plain `Enum`.""" + with pytest.raises(AttributeError): + _ = OptDeps.value # type: ignore[attr-defined] + + +def test_low_level_api_members_are_distinct() -> None: + """`chain_checks`-style values do not collapse either.""" + + class Chained(OptionalDependencyEnum): + A = get_version("packaging") + B = get_version("packaging") + C = get_version("this-is-not-a-package") + D = get_version("nor-is-this") + + assert len({Chained.A, Chained.B, Chained.C, Chained.D}) == 4 + assert Chained.A.installed and Chained.B.installed + assert not Chained.C.installed + assert not Chained.D.installed + assert is_installed("packaging") From 1a1993d8c8990d09cf4d6dbdc6e1c1602bb04bd4 Mon Sep 17 00:00:00 2001 From: nstarman Date: Thu, 6 Aug 2026 12:26:02 -0400 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=8E=A8=20style:=20split=20compound=20?= =?UTF-8?q?assertion=20for=20ruff=20PT018?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test file was untracked when `pre-commit run --all-files` last ran locally, so the hook skipped it and the lint only surfaced on CI. Co-Authored-By: Claude Opus 5 --- tests/test_no_aliasing.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_no_aliasing.py b/tests/test_no_aliasing.py index ae42919..a6263f0 100644 --- a/tests/test_no_aliasing.py +++ b/tests/test_no_aliasing.py @@ -114,7 +114,8 @@ class Chained(OptionalDependencyEnum): D = get_version("nor-is-this") assert len({Chained.A, Chained.B, Chained.C, Chained.D}) == 4 - assert Chained.A.installed and Chained.B.installed + assert Chained.A.installed + assert Chained.B.installed assert not Chained.C.installed assert not Chained.D.installed assert is_installed("packaging") From 3deee076ee7cfcd1f6323b6a100f34b07d371907 Mon Sep 17 00:00:00 2001 From: nstarman Date: Thu, 6 Aug 2026 12:29:18 -0400 Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=8E=A8=20style:=20reach=20for=20the?= =?UTF-8?q?=20public=20API=20where=20pylint=20flagged=20protected=20access?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_missing_` read `m._value_.resolved` and `__reduce_ex__` read `self._name_`; `value` and `name` say the same thing through the public descriptors, so this clears W0212/E1101 without a suppression. Co-Authored-By: Claude Opus 5 --- src/optional_dependencies/_core.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/optional_dependencies/_core.py b/src/optional_dependencies/_core.py index f11438c..7457da4 100644 --- a/src/optional_dependencies/_core.py +++ b/src/optional_dependencies/_core.py @@ -137,7 +137,7 @@ def _missing_(cls, value: object) -> "OptionalDependencyEnum | None": it always had: where several members share a version, the first one declared wins. """ - return next((m for m in cls if m._value_.resolved == value), None) + return next((m for m in cls if m.value == value), None) def __reduce_ex__(self, proto: object) -> tuple[object, ...]: """Pickle by name. @@ -147,7 +147,7 @@ def __reduce_ex__(self, proto: object) -> tuple[object, ...]: key besides: versions do not distinguish members, which is the whole problem being fixed here. """ - return getattr, (self.__class__, self._name_) + return getattr, (self.__class__, self.name) @DynamicClassAttribute def value(self) -> Version | Literal[InstalledState.NOT_INSTALLED]: From db4de5e0d3f074c04a014cfb56d6ee1022f75e47 Mon Sep 17 00:00:00 2001 From: nstarman Date: Thu, 6 Aug 2026 20:12:44 -0400 Subject: [PATCH 4/5] =?UTF-8?q?=E2=9C=85=20test:=20cover=20`=5FMemberKey.?= =?UTF-8?q?=5F=5Frepr=5F=5F`,=20restoring=20100%=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `main` is at 100%; the branch had dropped to 99% on a single uncovered line -- `_MemberKey.__repr__`, added by this PR. The delegation it performs is the thing keeping members rendering as `>` rather than leaking the wrapper, which the PR claims but nothing asserted. Checks the version's repr appears and the wrapper's name does not, rather than matching `Enum.__repr__`'s exact format, so the test does not break if CPython reformats it. Co-Authored-By: Claude Opus 5 --- tests/test_no_aliasing.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_no_aliasing.py b/tests/test_no_aliasing.py index a6263f0..0ce813d 100644 --- a/tests/test_no_aliasing.py +++ b/tests/test_no_aliasing.py @@ -58,6 +58,14 @@ def test_value_is_still_the_version() -> None: assert OptDeps.NOTINSTALLED.value is NOT_INSTALLED +@pytest.mark.parametrize("name", ["PACKAGING", "NOTINSTALLED"]) +def test_repr_delegates_to_the_version(name: str) -> None: + """The key wrapper is invisible: a member still renders as its version.""" + member = OptDeps[name] + assert repr(member.value) in repr(member) + assert "_MemberKey" not in repr(member) + + def test_members_are_hashable_and_usable_as_keys() -> None: """Distinct members occupy distinct slots in a dict/set.""" mapping = {m: m.name for m in OptDeps} From 4d2ae2155fa61a5bc0b5f71856df414e74d0ee7c Mon Sep 17 00:00:00 2001 From: nstarman Date: Thu, 6 Aug 2026 20:15:30 -0400 Subject: [PATCH 5/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor:=20make=20`?= =?UTF-8?q?=5FMemberKey`=20a=20frozen=20slotted=20dataclass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Matches `Comparator` directly above, drops the hand-written `__init__`, and `frozen` now enforces what was only convention -- the key is set once during member construction and must never move afterwards. `eq=False` is required, not stylistic. `dataclass` generates `__eq__` from fields by default, so two keys wrapping equal versions would compare equal and `enum` would fold their members back into aliases -- the exact bug this PR fixes. Verified: with the default `eq=True`, the seven tests that pin the fix fail again. Noted at the decorator, since that is where someone would delete it. `repr=False` likewise, so the delegating `__repr__` survives and members keep rendering as their version rather than as `_MemberKey(resolved=...)`. Co-Authored-By: Claude Opus 5 --- src/optional_dependencies/_core.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/optional_dependencies/_core.py b/src/optional_dependencies/_core.py index 7457da4..25a345f 100644 --- a/src/optional_dependencies/_core.py +++ b/src/optional_dependencies/_core.py @@ -78,6 +78,11 @@ def __call__(self, enum: "OptionalDependencyEnum", other: object) -> bool: @final +# `eq=False` is load-bearing, not tidiness: the generated `__eq__` compares by +# field, so two keys wrapping equal versions would compare equal and `enum` +# would alias their members straight back together. `repr=False` leaves the +# delegating `__repr__` below in place. +@dataclass(frozen=True, slots=True, eq=False, repr=False) class _MemberKey: """The internal ``_value_`` of an `OptionalDependencyEnum` member. @@ -95,12 +100,7 @@ class _MemberKey: delegates so members still show as ``>``. """ - __slots__ = ("resolved",) - - def __init__( - self, resolved: Version | Literal[InstalledState.NOT_INSTALLED], / - ) -> None: - self.resolved = resolved + resolved: Version | Literal[InstalledState.NOT_INSTALLED] def __repr__(self) -> str: return repr(self.resolved)