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 170ff00..466fb26 100644 --- a/src/optional_dependencies/_core.py +++ b/src/optional_dependencies/_core.py @@ -7,14 +7,18 @@ import operator from dataclasses import dataclass from enum import Enum -from types import MethodType -from typing import Callable, Literal, cast +from types import DynamicClassAttribute, MethodType +from typing import Callable, 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 branch supports 3.9, and +#: `typing_extensions` is not a dependency. +_EnumT = TypeVar("_EnumT", bound="OptionalDependencyEnum") + @dataclass(frozen=True) class Comparator: @@ -74,9 +78,103 @@ 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 ``>``. + + Written out rather than a `dataclasses.dataclass` as on ``main``: this + branch supports Python 3.9, where ``dataclass`` has no ``slots``. The + generated ``__eq__`` would have to be turned off anyway -- comparing by + field is exactly the aliasing this class exists to prevent. + """ + + __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.9+) 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 == 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..0ce813d --- /dev/null +++ b/tests/test_no_aliasing.py @@ -0,0 +1,129 @@ +"""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 + + +@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} + 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 + assert Chained.B.installed + assert not Chained.C.installed + assert not Chained.D.installed + assert is_installed("packaging")