Skip to content

Commit

Permalink
Various improvements.
Browse files Browse the repository at this point in the history
- Use positional-only syntax rather than typing.overload's for various
  methods now that 3.7 is no longer supported. This works around
  mypyc/mypyc#961 but is also preferable on its own merits.

- Other minor changes to enable `mypyc _base.py` to successfully
  compile, such as not assuming `BidictBase.__dict__`.

- Use `typing.override` on 3.12.

- Prefer `alias = t.Tuple[...]` over `alias = 'tuple[...]'`, because
  this will allow ruff's pyupgrade rules to automatically rewrite these
  to `alias = tuple[...]` once we drop support for 3.8.

- Other minor code, docs, and test improvements.

CI:

- Enable coverage for 3.11 now that we use typing.override on 3.12+ and
  use a dummy impl on <3.12.

- Increase max-examples for the "more-examples" hypothesis profile to 999.
  With max-examples=500, the latest scheduled test run on GitHub Actions
  took under 5 minutes, so we can afford to increase this a lot more.

- Use the "more-examples" profile for all jobs in the test matrix for
  scheduled test runs, since we can afford it.
  • Loading branch information
jab committed Jan 12, 2024
1 parent 67f0593 commit 734622b
Show file tree
Hide file tree
Showing 15 changed files with 158 additions and 130 deletions.
9 changes: 6 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ jobs:
- pyversion: "3.12"
enable_coverage: true
- pyversion: "3.11"
enable_coverage: true
- pyversion: "3.10"
more_hypothesis_examples_if_cron: true
- pyversion: "3.9"
- pyversion: "3.8"
- pyversion: "pypy-3.10"
Expand All @@ -47,7 +47,7 @@ jobs:
- run: python -m pip install -U pip setuptools wheel tox==4.11.4
- run: python -m pip install -r dev-deps/${{ matrix.deps_subdir || format('python{0}', matrix.pyversion) }}/test.txt
- name: maybe set --hypothesis-profile=more-examples # See tests/conftest.py
if: ${{ github.event_name == 'schedule' && matrix.more_hypothesis_examples_if_cron }}
if: ${{ github.event_name == 'schedule' }}
run: |
echo PYTEST_ADDOPTS="${PYTEST_ADDOPTS} --hypothesis-profile=more-examples" >> "${GITHUB_ENV}"
- name: cache .hypothesis dir
Expand All @@ -58,9 +58,12 @@ jobs:
- name: maybe enable coverage
if: matrix.enable_coverage
run: |
echo COVERAGE_CORE=sysmon >> "${GITHUB_ENV}"
echo COVERAGE_PROCESS_START="$(pwd)/.coveragerc" >> "${GITHUB_ENV}"
echo RUN_PYTEST_CMD="coverage run" >> "${GITHUB_ENV}"
- name: set COVERAGE_CORE=sysmon if py3.12
if: matrix.pyversion == '3.12'
run: |
echo COVERAGE_CORE=sysmon >> "${GITHUB_ENV}"
- run: ${RUN_PYTEST_CMD:-python} -m pytest
- name: combine and show any collected coverage
if: matrix.enable_coverage
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
*.pyc
*.so
__pycache__
.DS_Store
.benchmarks
Expand Down
18 changes: 12 additions & 6 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ please consider sponsoring bidict on GitHub.`
0.23.0 (not yet released)
-------------------------

Primarily, this release simplifies bidict by removing features
that few if any users depend on.

It also contains a few other minor improvements.
Primarily, this release simplifies bidict by removing minor features
that are no longer necessary or evidently are substantially unused.
These simplifications will make it easier to improve bidict in the future,
including further potential performance improvements.
It also contains several other minor improvements.

- Drop support for Python 3.7,
which reached end of life on 2023-06-27,
Expand All @@ -53,8 +54,13 @@ It also contains a few other minor improvements.
as well as
:ref:`just value duplication <basic-usage:values must be unique>`.

- Fix a bug where e.g. ``bidict(None)`` would incorrectly return an empty bidict
rather than raising :class:`TypeError`.
- Improve type hints for the
:attr:`~bidict.BidictBase.inv` shortcut alias
for :attr:`~bidict.BidictBase.inverse`.

- Fix a bug where calls like
``bidict(None)``, ``bi.update(False)``, etc.
would fail to raise a :class:`TypeError`.

- All :meth:`~bidict.bidict.__init__`,
:meth:`~bidict.bidict.update`,
Expand Down
2 changes: 1 addition & 1 deletion bidict/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
from ._exc import KeyAndValueDuplicationError as KeyAndValueDuplicationError
from ._exc import KeyDuplicationError as KeyDuplicationError
from ._exc import ValueDuplicationError as ValueDuplicationError
from ._frozenbidict import frozenbidict as frozenbidict
from ._frozen import frozenbidict as frozenbidict
from ._iter import inverted as inverted
from ._orderedbase import OrderedBidictBase as OrderedBidictBase
from ._orderedbidict import OrderedBidict as OrderedBidict
Expand Down
90 changes: 48 additions & 42 deletions bidict/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
# * Code review nav *
# (see comments in __init__.py)
# ============================================================================
# ← Prev: _abc.py Current: _base.py Next: _frozenbidict.py →
# ← Prev: _abc.py Current: _base.py Next: _frozen.py →
# ============================================================================


Expand Down Expand Up @@ -40,16 +40,16 @@
from ._typing import OKT
from ._typing import OVT
from ._typing import VT
from ._typing import Items
from ._typing import Maplike
from ._typing import MapOrItems
from ._typing import override


OldKV: t.TypeAlias = 'tuple[OKT[KT], OVT[VT]]'
DedupResult: t.TypeAlias = 'OldKV[KT, VT] | None'
Write: t.TypeAlias = 'list[t.Callable[[], None]]'
OldKV: t.TypeAlias = t.Tuple[OKT[KT], OVT[VT]]
DedupResult: t.TypeAlias = t.Optional[OldKV[KT, VT]]
Write: t.TypeAlias = t.List[t.Callable[[], None]]
Unwrite: t.TypeAlias = Write
PreparedWrite: t.TypeAlias = 'tuple[Write, Unwrite]'
PreparedWrite: t.TypeAlias = t.Tuple[Write, Unwrite]
BT = t.TypeVar('BT', bound='BidictBase[t.Any, t.Any]')


Expand All @@ -60,16 +60,6 @@ class BidictKeysView(t.KeysView[KT], t.ValuesView[KT]):
"""


def get_arg(*args: MapOrItems[KT, VT]) -> MapOrItems[KT, VT]:
"""Ensure there's at most one arg in *args* and that it is iterable, then return it."""
if len(args) > 1:
raise TypeError(f'Expected at most 1 positional argument, got {len(args)}')
arg = args[0] if args else ()
if not isinstance(arg, (t.Iterable, Maplike)):
raise TypeError(f"'{arg.__class__.__name__}' object is not iterable")
return arg


class BidictBase(BidirectionalMapping[KT, VT]):
"""Base class implementing :class:`BidirectionalMapping`."""

Expand Down Expand Up @@ -101,7 +91,7 @@ def _init_class(cls) -> None:
cls._ensure_inv_cls()
cls._set_reversed()

__reversed__: t.Any
__reversed__: t.ClassVar[t.Any]

@classmethod
def _set_reversed(cls) -> None:
Expand All @@ -113,24 +103,33 @@ def _set_reversed(cls) -> None:
overridden = resolved is not BidictBase.__reversed__
if overridden: # E.g. OrderedBidictBase, OrderedBidict
return
# The following will be False for MutableBidict, bidict, and frozenbidict on Python < 3.8,
# and True for them on 3.8+ (where dicts are reversible). Will also be True for custom
# subclasses like SortedBidict (see https://bidict.rtfd.io/extending.html#sortedbidict-recipes).
backing_reversible = all(issubclass(i, t.Reversible) for i in (cls._fwdm_cls, cls._invm_cls))
cls.__reversed__ = _fwdm_reversed if backing_reversible else None

@classmethod
def _ensure_inv_cls(cls) -> None:
"""Ensure :attr:`_inv_cls` is set, computing it dynamically if necessary.
All subclasses provided in :mod:`bidict` are their own inverse classes,
i.e., their backing forward and inverse mappings are both the same type,
but users may define subclasses where this is not the case.
This method ensures that the inverse class is computed correctly regardless.
See: :ref:`extending:Dynamic Inverse Class Generation`
(https://bidict.rtfd.io/extending.html#dynamic-inverse-class-generation)
All subclasses provided in :mod:`bidict` are their own inverse classes,
but custom, user-defined subclasses may have distinct inverse classes.
"""
if cls.__dict__.get('_inv_cls'):
return # Already set, nothing to do.
# This _ensure_inv_cls() method is (indirectly) corecursive with _make_inv_cls() below
# in the case that we need to dynamically generate the inverse class:
# 1. _ensure_inv_cls() calls cls._make_inv_cls()
# 2. cls._make_inv_cls() calls type(..., (cls, ...), ...) to dynamically generate inv_cls
# 3. Our __init_subclass__ hook (see above) is automatically called on inv_cls
# 4. inv_cls.__init_subclass__() calls inv_cls._ensure_inv_cls()
# 5. inv_cls._ensure_inv_cls() resolves to this implementation
# (inv_cls deliberately does not override this), so we're back where we started.
# But since the _make_inv_cls() call will have set inv_cls.__dict__._inv_cls,
# just check if it's already set before calling _make_inv_cls() to prevent infinite recursion.
if getattr(cls, '__dict__', {}).get('_inv_cls'): # Don't assume cls.__dict__ (e.g. mypyc native class)
return
cls._inv_cls = cls._make_inv_cls()

@classmethod
Expand All @@ -153,26 +152,22 @@ def _inv_cls_dict_diff(cls) -> dict[str, t.Any]:
'_invm_cls': cls._fwdm_cls,
}

@t.overload
def __init__(self, **kw: VT) -> None: ...
@t.overload
def __init__(self, __m: Maplike[KT, VT], **kw: VT) -> None: ...
@t.overload
def __init__(self, __i: Items[KT, VT], **kw: VT) -> None: ...
def __init__(self, *args: MapOrItems[KT, VT], **kw: VT) -> None:
def __init__(self, arg: MapOrItems[KT, VT] = (), /, **kw: VT) -> None:
"""Make a new bidirectional mapping.
The signature behaves like that of :class:`dict`.
Items passed in are added in the order they are passed,
respecting the :attr:`on_dup` class attribute in the process.
ktems passed via positional arg are processed first,
followed by any items passed via keyword argument.
Any duplication encountered along the way
is handled as per :attr:`on_dup`.
"""
self._fwdm = self._fwdm_cls()
self._invm = self._invm_cls()
if args or kw:
self._update(get_arg(*args), kw, rbof=False)
self._update(arg, kw, rbof=False)

# If Python ever adds support for higher-kinded types, `inverse` could use them, e.g.
# def inverse(self: BT[KT, VT]) -> BT[VT, KT]:
# Ref: https://github.com/python/typing/issues/548#issuecomment-621571821
@override
@property
def inverse(self) -> BidictBase[VT, KT]:
"""The inverse of this bidirectional mapping instance."""
Expand Down Expand Up @@ -223,6 +218,7 @@ def __repr__(self) -> str:
items = dict(self.items()) if self else ''
return f'{clsname}({items})'

@override
def values(self) -> BidictKeysView[VT]:
"""A set-like object providing a view on the contained values.
Expand All @@ -238,6 +234,7 @@ def values(self) -> BidictKeysView[VT]:
"""
return t.cast(BidictKeysView[VT], self.inverse.keys())

@override
def keys(self) -> t.KeysView[KT]:
"""A set-like object providing a view on the contained keys.
Expand All @@ -255,6 +252,7 @@ def keys(self) -> t.KeysView[KT]:
fwdm = self._fwdm
return fwdm.keys() if isinstance(fwdm, dict) else BidictKeysView(self)

@override
def items(self) -> t.ItemsView[KT, VT]:
"""A set-like object providing a view on the contained items.
Expand All @@ -274,13 +272,15 @@ def items(self) -> t.ItemsView[KT, VT]:
# The inherited collections.abc.Mapping.__contains__() method is implemented by doing a `try`
# `except KeyError` around `self[key]`. The following implementation is much faster,
# especially in the missing case.
@override
def __contains__(self, key: t.Any) -> bool:
"""True if the mapping contains the specified key, else False."""
return key in self._fwdm

# The inherited collections.abc.Mapping.__eq__() method is implemented in terms of an inefficient
# `dict(self.items()) == dict(other.items())` comparison, so override it with a
# more efficient implementation.
@override
def __eq__(self, other: object) -> bool:
"""*x.__eq__(other) ⟺ x == other*
Expand Down Expand Up @@ -431,7 +431,8 @@ def _update(
on_dup: OnDup | None = None,
) -> None:
"""Update, possibly rolling back on failure as per *rbof*."""
# Must process input in a single pass, since arg may be a generator.
if not isinstance(arg, (t.Iterable, Maplike)):
raise TypeError(f"'{arg.__class__.__name__}' object is not iterable")
if not arg and not kw:
return
if on_dup is None:
Expand All @@ -447,6 +448,8 @@ def _update(
# that does not inherit from BidictBase, it's a foreign implementation, so we
# perform duplication checking to err on the safe side.)

# Note: We must process input in a single pass, since arg may be a generator.

# If we roll back on failure and we know that there are more updates to process than
# already-contained items, our rollback strategy is to update a copy of self (without
# rolling back on failure), and then to become the copy if all updates succeed.
Expand Down Expand Up @@ -480,6 +483,10 @@ def _update(
if rbof and unwrite: # save the unwrite for later application if needed
append_unwrite(unwrite)

def __copy__(self: BT) -> BT:
"""Used for the copy protocol. See the :mod:`copy` module."""
return self.copy()

def copy(self: BT) -> BT:
"""Make a (shallow) copy of this bidict."""
# Could just `return self.__class__(self)` here, but the below is faster. The former
Expand Down Expand Up @@ -509,10 +516,6 @@ def _init_from(self, other: MapOrItems[KT, VT]) -> None:
inv = other.inverse if isinstance(other, BidictBase) else inverted(self._fwdm)
self._invm.update(inv)

#: Used for the copy protocol.
#: *See also* the :mod:`copy` module
__copy__ = copy

# other's type is Mapping rather than Maplike since bidict() | SupportsKeysAndGetItem({})
# raises a TypeError, just like dict() | SupportsKeysAndGetItem({}) does.
def __or__(self: BT, other: t.Mapping[KT, VT]) -> BT:
Expand All @@ -531,14 +534,17 @@ def __ror__(self: BT, other: t.Mapping[KT, VT]) -> BT:
new._update(self, rbof=False)
return new

@override
def __len__(self) -> int:
"""The number of contained items."""
return len(self._fwdm)

@override
def __iter__(self) -> t.Iterator[KT]:
"""Iterator over the contained keys."""
return iter(self._fwdm)

@override
def __getitem__(self, key: KT) -> VT:
"""*x.__getitem__(key) ⟺ x[key]*"""
return self._fwdm[key]
Expand Down Expand Up @@ -572,5 +578,5 @@ class GeneratedBidictInverse:

# * Code review nav *
# ============================================================================
# ← Prev: _abc.py Current: _base.py Next: _frozenbidict.py →
# ← Prev: _abc.py Current: _base.py Next: _frozen.py →
# ============================================================================
Loading

0 comments on commit 734622b

Please sign in to comment.