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

implement stricter 1-to-1 checking #26

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from 16 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions .travis.yml
@@ -1,6 +1,7 @@
language: python
python:
- "2.7"
- "3.3"
- "3.4"
- "3.5"
- "pypy"
Expand All @@ -12,8 +13,7 @@ install:
- pip install tox-travis

script:
- tox
- py.test --cov bidict tests
- python setup.py test

after_success:
coveralls
Expand Down
7 changes: 4 additions & 3 deletions CONTRIBUTING.rst
Expand Up @@ -107,12 +107,13 @@ Besides creating issues and pull requests, there are other ways to contribute.
If you read the code and learned something new, let us know and it'll give us the warm fuzzies.
If you're using bidict in a project you work on, blog about your experience.
If you come across other people who could find it useful, spread the word.
If bidict has helped you accomplish work you've been paid for,
please `support bidict <https://gumroad.com/l/XGXTp>`_
If bidict has helped you accomplish your work,
especially work you've been paid for,
please `support bidict <https://gumroad.com/l/bidict>`_
and/or ask your organization to do the same.

.. image:: ./docs/_static/support-on-gumroad.png
:target: https://gumroad.com/l/XGXTp
:target: https://gumroad.com/l/bidict
:alt: Support bidict

You can also use `Bountysource <https://www.bountysource.com/teams/jab>`_
Expand Down
7 changes: 4 additions & 3 deletions README.rst
Expand Up @@ -4,6 +4,7 @@ bidict
Efficient, Pythonic bidirectional map implementation and related functionality.

.. image:: ./docs/_static/logo.png
:target: https://bidict.readthedocs.org/
:alt: bidict logo


Expand Down Expand Up @@ -47,7 +48,7 @@ Status
:alt: License

.. image:: https://img.shields.io/badge/gumroad-support%20bidict-brightgreen.svg
:target: https://gum.co/XGXTp
:target: https://gumroad.com/l/bidict
:alt: Support bidict

.. image:: https://img.shields.io/badge/Paypal-Buy%20a%20Drink-blue.svg
Expand Down Expand Up @@ -86,12 +87,12 @@ talk about your use case.

bidict is the result of hundreds of hours of voluntary, unpaid work.
If bidict has helped you accomplish work you've been paid for,
please `support bidict <https://gumroad.com/l/XGXTp>`_
please `support bidict <https://gumroad.com/l/bidict>`_
and/or ask your organization to do the same.
Your support directly contributes to bidict's sustainability.

.. image:: ./docs/_static/support-on-gumroad.png
:target: https://gumroad.com/l/XGXTp
:target: https://gumroad.com/l/bidict
:alt: Support bidict

Check out
Expand Down
2 changes: 1 addition & 1 deletion bidict/VERSION
@@ -1 +1 @@
0.10.0.dev
0.10.0.dev0
9 changes: 5 additions & 4 deletions bidict/__init__.py
Expand Up @@ -9,18 +9,19 @@

"""

from ._common import BidirectionalMapping, CollapseException
from ._common import BidirectionalMapping, BidictException, ValueExistsException
from ._bidict import bidict
from ._collapsing import collapsingbidict
from ._loose import loosebidict
from ._frozen import frozenbidict
from ._named import namedbidict
from .util import pairs, inverted

__all__ = (
'BidirectionalMapping',
'CollapseException',
'BidictException',
'ValueExistsException',
'bidict',
'collapsingbidict',
'loosebidict',
'frozenbidict',
'namedbidict',
'pairs',
Expand Down
52 changes: 37 additions & 15 deletions bidict/_bidict.py
Expand Up @@ -13,32 +13,31 @@ def _del(self, key):
del self._bwd[val]

def __delitem__(self, key):
"""
Analogous to dict.__delitem__(), keeping bidirectionality intact.
"""
self._del(key)

def __setitem__(self, key, val):
"""
Analogous to dict.__setitem__(), keeping bidirectionality intact.

:raises ValueExistsException: if attempting to insert a mapping with a
non-unique value.
"""
self._put(key, val)

def put(self, key, val):
"""
Alternative to using the setitem syntax to insert a mapping.
Alternative to using :attr:`__setitem__` to insert a mapping.
"""
self._put(key, val)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it may now be sensible for put() to be the "non-overwriting set", rather than an alias for __setitem__. For example,

def put(self, key, val):
    if (key in self._fwd and self._fwd[key] != val) or (val in self._bwd and self._bwd[val] != key):
        raise ItemExistsException(something)
    self._put(key, val)

This closes off the case you struggle to explain in the Caveats docs, about how to insert an item without a risk of accidental overwrite on the key side as well as the value side. Meanwhile the __setitem__ syntax on the forward and inverse bidicts allows overwrites on keys and values respectively, and forceput() allows overrides from both ends.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Another great suggestion. Implemented in de821e9.


def forceput(self, key, val):
"""
Like :attr:`bidict.bidict.put` but silently removes any existing
mapping that would otherwise cause a :class:`bidict.CollapseException`
before inserting the given mapping::

>>> b = bidict({0: 'zero', 1: 'one'})
>>> b.put(0, 'one') # doctest: +IGNORE_EXCEPTION_DETAIL
Traceback (most recent call last):
...
CollapseException: ((0, 'zero'), (1, 'one'))
>>> b.forceput(0, 'one')
>>> b
bidict({0: 'one'})

Like :attr:`put`, but silently removes any existing
mapping that would otherwise cause :class:`ValueExistsException`
before inserting the given mapping.
"""
oldval = self._fwd.get(key, _missing)
oldkey = self._bwd.get(val, _missing)
Expand All @@ -50,26 +49,49 @@ def forceput(self, key, val):
self._bwd[val] = key

def clear(self):
"""
Removes all items.
"""
self._fwd.clear()
self._bwd.clear()

def pop(self, key, *args):
"""
Analogous to dict.pop(), keeping bidirectionality intact.
"""
val = self._fwd.pop(key, *args)
del self._bwd[val]
return val

def popitem(self):
"""
Analogous to dict.popitem(), keeping bidirectionality intact.
"""
if not self._fwd:
raise KeyError('popitem(): %s is empty' % self.__class__.__name__)
key, val = self._fwd.popitem()
del self._bwd[val]
return key, val

def setdefault(self, key, default=None):
"""
Analogous to dict.setdefault(), keeping bidirectionality intact.
"""
val = self._fwd.setdefault(key, default)
self._bwd[val] = key
return val

def update(self, *args, **kw):
"""
Analogous to dict.update(), keeping bidirectionality intact.
"""
return self._update(*args, **kw)

def forceupdate(self, *args, **kw):
"""
Like :attr:`update`, but silently removes any existing
mappings that would otherwise cause :class:`ValueExistsException`
allowing key-overwriting updates to succeed.
"""
for k, v in pairs(*args, **kw):
self._put(k, v)
self.forceput(k, v)
9 changes: 0 additions & 9 deletions bidict/_collapsing.py

This file was deleted.

72 changes: 36 additions & 36 deletions bidict/_common.py
@@ -1,4 +1,4 @@
from .compat import PY2, iteritems, viewitems
from .compat import PY2, iteritems
from .util import pairs
from collections import Mapping

Expand All @@ -7,13 +7,12 @@ class BidirectionalMapping(Mapping):
"""
Mutable and immutable bidict types extend this class,
which implements all the shared logic.
Users typically won't need to touch this.
Users will typically only interact with subclasses of this class.
"""
def __init__(self, *args, **kw):
self._fwd = {}
self._bwd = {}
for (k, v) in pairs(*args, **kw):
self._put(k, v)
self._update(*args, **kw)
inv = object.__new__(self.__class__)
inv._fwd = self._bwd
inv._bwd = self._fwd
Expand Down Expand Up @@ -41,53 +40,49 @@ def inv(self):
return self._inv

def __inverted__(self):
"""
Returns an iterator over the inverse mappings.
"""
return iteritems(self._bwd)

def __getitem__(self, key):
return self._fwd[key]

def _put(self, key, val):
try:
oldval = self._fwd[key]
except KeyError:
oldval = _missing
try:
oldkey = self._bwd[val]
except KeyError:
oldkey = _missing

if oldval is not _missing and oldkey is not _missing:
if key == oldkey and val == oldval:
return
raise CollapseException((key, oldval), (oldkey, val))
elif oldval is not _missing:
del self._bwd[oldval]
elif oldkey is not _missing:
del self._fwd[oldkey]

oldkey = self._bwd.get(val, _missing)
oldval = self._fwd.get(key, _missing)
if key == oldkey and val == oldval:
return
if oldkey is not _missing:
raise ValueExistsException((oldkey, val))
self._fwd[key] = val
self._bwd[val] = key
self._bwd.pop(oldval, None)

def _update(self, *args, **kw):
for k, v in pairs(*args, **kw):
self._put(k, v)

get = lambda self, k, *args: self._fwd.get(k, *args)
copy = lambda self: self.__class__(self._fwd)
get.__doc__ = dict.get.__doc__
copy.__doc__ = dict.copy.__doc__
get.__doc__ = 'Analogous to dict.get()'
copy.__doc__ = 'Analogous to dict.copy()'
__len__ = lambda self: len(self._fwd)
__iter__ = lambda self: iter(self._fwd)
__contains__ = lambda self, x: x in self._fwd
__len__.__doc__ = dict.__len__.__doc__
__iter__.__doc__ = dict.__iter__.__doc__
__contains__.__doc__ = dict.__contains__.__doc__
__len__.__doc__ = 'Analogous to dict.__len__()'
__iter__.__doc__ = 'Analogous to dict.__iter__()'
__contains__.__doc__ = 'Analogous to dict.__contains__()'
keys = lambda self: self._fwd.keys()
items = lambda self: self._fwd.items()
keys.__doc__ = dict.keys.__doc__
items.__doc__ = dict.items.__doc__
keys.__doc__ = 'Analogous to dict.keys()'
items.__doc__ = 'Analogous to dict.items()'
values = lambda self: self._bwd.keys()
values.__doc__ = \
"D.values() -> a set-like object providing a view on D's values. " \
'Note that because values of a BidirectionalMapping are also keys, ' \
'this returns a ``dict_keys`` object rather than a ``dict_values`` ' \
'object.'
"B.values() -> a set-like object providing a view on B's values. " \
'Note that because values of a BidirectionalMapping are also keys ' \
'of its inverse, this returns a ``dict_keys`` object rather than a ' \
'``dict_values`` object, conferring set-like benefits.'
if PY2:
iterkeys = lambda self: self._fwd.iterkeys()
viewkeys = lambda self: self._fwd.viewkeys()
Expand All @@ -104,11 +99,16 @@ def _put(self, key, val):
values.__doc__ = dict.values.__doc__


class CollapseException(Exception):
class BidictException(Exception):
"""
Raised when an attempt is made to insert a new mapping into a bidict that
would collapse two existing mappings.
Base class for bidict exceptions.
"""

class ValueExistsException(BidictException):
"""
Raised when an attempt is made to insert a new mapping into a bidict whose
key maps to the value of an existing mapping, violating the uniqueness
constraint.
"""

_missing = object()
9 changes: 9 additions & 0 deletions bidict/_loose.py
@@ -0,0 +1,9 @@
from ._bidict import bidict

class loosebidict(bidict):
"""
A mutable bidict which always uses forcing put operations
so that it never raises :class:`ValueExistsException`.
"""
def _put(self, key, val):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tiny performance optimisation, but

_put = bidict.forceput

would remove function call overhead without increasing code duplication. I imagine _put() will get called a lot in loops, so this adds up.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Originally had that but changed it in favor of dynamic method lookup for more extensibility. Worth it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right. We shouldn't make changes like this unless we've profiled what the possible speedups might be.

return self.forceput(key, val)
24 changes: 5 additions & 19 deletions bidict/util.py
Expand Up @@ -34,32 +34,18 @@ def pairs(*map_or_it, **kw):

class inverted(Iterator):
"""
An iterator analogous to the :func:`reversed` built-in.
Useful for inverting a mapping.
An iterator yielding the inverses of the provided mappings.
Works with any object that can be iterated over as a mapping or in pairs
or that implements its own __inverted__ method.
"""
def __init__(self, data):
self._data = data

def __iter__(self):
"""
First try to call ``__inverted__`` on the wrapped object
and return the result if the call succeeds
(i.e. delegate to the object if it supports inverting natively).
This complements :attr:`bidict.BidirectionalMapping.__inverted__`.

If the call fails, fall back on calling our own ``__next__`` method.
"""
try:
it = self._data.__inverted__
except AttributeError:
it = self.__next__
return it()
makeit = getattr(self._data, '__inverted__', self.__next__)
return makeit()

def __next__(self):
"""
Yields the inverse of each pair yielded by calling :attr:`bidict.pairs`
on the wrapped object.
"""
for (k, v) in pairs(self._data):
yield (v, k)

Expand Down
Binary file removed docs/_static/bidict_types.png
Binary file not shown.
4 changes: 4 additions & 0 deletions docs/_static/class-hierarchy.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.