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

Fully typed RangeMap and avoid complete iterations to find matches #16

Merged
merged 8 commits into from
Aug 25, 2024
46 changes: 30 additions & 16 deletions jaraco/collections/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,17 @@
import random
import re
from collections.abc import Container, Iterable, Mapping
from typing import Any, Callable, Union
from typing import TYPE_CHECKING, Any, Callable, Dict, TypeVar, Union, overload

import jaraco.text

if TYPE_CHECKING:
from _typeshed import SupportsKeysAndGetItem
from typing_extensions import Self

_T = TypeVar('_T')
_VT = TypeVar('_VT')

_Matchable = Union[Callable, Container, Iterable, re.Pattern]


Expand Down Expand Up @@ -119,7 +126,7 @@ def dict_map(function, dictionary):
return dict((key, function(value)) for key, value in dictionary.items())


class RangeMap(dict):
class RangeMap(Dict[int, _VT]):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. I didn't know you could subclass from Dict. TIL.

int is overspecified here. RangeMap accepts any keys (possibly heterogeneous) that are comparable based on the key_match_comparator.

Also, the values don't need to be uniform. They can be heterogeneous.

Copy link
Contributor Author

@Avasam Avasam Jul 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. I didn't know you could subclass from Dict. TIL.

Yeah typing re-exports runtime generic aliases of builtin and collection types. It's useful when trying to specify generic parameters for builtins and collections that are not yet subscriptable in older Python versions.

Also, the values don't need to be uniform. They can be heterogeneous.

Is RangeMap meant to be mutable? If not I think it would make sense to make the value covariant like a Mapping

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is RangeMap meant to be mutable?

I was not intending for it to be restricted to be immutable. I can't recall if I've used it for an application that changes it dynamically, but it's a use-case that should be supported unless there's a practical benefit to making it immutable. It was modeled after the dict, so is meant to have similar flexibility.

Copy link
Contributor Author

@Avasam Avasam Jul 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, in that case I think it's correct to keep both keys and values invariant.

"""
A dictionary-like object that uses the keys as bounds for a range.
Inclusion of the value for that range is determined by the
Expand Down Expand Up @@ -186,7 +193,7 @@ class RangeMap(dict):
which requires use of sort params and a key_match_comparator.

>>> r = RangeMap({1: 'a', 4: 'b'},
... sort_params=dict(reverse=True),
... sort_params={'reverse': True},
Avasam marked this conversation as resolved.
Show resolved Hide resolved
... key_match_comparator=operator.ge)
>>> r[1], r[2], r[3], r[4], r[5], r[6]
('a', 'a', 'a', 'b', 'b', 'b')
Expand All @@ -202,21 +209,23 @@ class RangeMap(dict):

def __init__(
self,
source,
source: SupportsKeysAndGetItem[int, _VT] | Iterable[tuple[int, _VT]],
sort_params: Mapping[str, Any] = {},
key_match_comparator=operator.le,
key_match_comparator: Callable[[int, int], bool] = operator.le,
):
dict.__init__(self, source)
self.sort_params = sort_params
self.match = key_match_comparator

@classmethod
def left(cls, source):
def left(
cls, source: SupportsKeysAndGetItem[int, _VT] | Iterable[tuple[int, _VT]]
) -> Self:
return cls(
source, sort_params=dict(reverse=True), key_match_comparator=operator.ge
source, sort_params={'reverse': True}, key_match_comparator=operator.ge
)

def __getitem__(self, item):
def __getitem__(self, item: int) -> _VT:
sorted_keys = sorted(self.keys(), **self.sort_params)
if isinstance(item, RangeMap.Item):
result = self.__getitem__(sorted_keys[item])
Expand All @@ -227,7 +236,11 @@ def __getitem__(self, item):
raise KeyError(key)
return result

def get(self, key, default=None):
@overload # type: ignore[override] # Signature simplified over dict and Mapping
def get(self, key: int, default: _T) -> _VT | _T: ...
@overload
def get(self, key: int, default: None = None) -> _VT | None: ...
def get(self, key: int, default: _T | None = None) -> _VT | _T | None:
"""
Return the value for key if key is in the dictionary, else default.
If default is not given, it defaults to None, so that this method
Expand All @@ -238,22 +251,23 @@ def get(self, key, default=None):
except KeyError:
return default

def _find_first_match_(self, keys, item):
def _find_first_match_(self, keys: Iterable[int], item: int) -> int:
is_match = functools.partial(self.match, item)
matches = list(filter(is_match, keys))
if matches:
return matches[0]
raise KeyError(item)
matches = filter(is_match, keys)
try:
return next(matches)
except StopIteration:
raise KeyError(item) from None

def bounds(self):
def bounds(self) -> tuple[int, int]:
sorted_keys = sorted(self.keys(), **self.sort_params)
return (sorted_keys[RangeMap.first_item], sorted_keys[RangeMap.last_item])

# some special values for the RangeMap
undefined_value = type('RangeValueUndefined', (), {})()

class Item(int):
"RangeMap Item"
'RangeMap Item'
jaraco marked this conversation as resolved.
Show resolved Hide resolved

first_item = Item(0)
last_item = Item(-1)
Expand Down
1 change: 1 addition & 0 deletions newsfragments/16.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fully typed ``RangeMap`` and avoid complete iterations to find matches -- by :user:`Avasam`
Loading