Repository: mlenzen/collections-extended

Issue:
Rename `d` arguments to `default` in indexed_dict

Discussion:

Patch:
diff --git a/collections_extended/_util.py b/collections_extended/_util.py
index f5a81f8..84050a5 100644
--- a/collections_extended/_util.py
+++ b/collections_extended/_util.py
@@ -67,6 +67,11 @@ class Sentinel(object):
 NOT_SET = Sentinel('not_set')
 
 
+def deprecation_warning(msg):
+	"""Raise a deprecation warning."""
+	warnings.warn(msg, category=DeprecationWarning, stacklevel=2)
+
+
 def deprecated(msg, dep_version):
 	"""Decorate a function, method or class to mark as deprecated.
 
@@ -96,7 +101,7 @@ def deprecated(msg, dep_version):
 
 		@wraps(func)
 		def inner(*args, **kwargs):
-			warnings.warn(msg, category=DeprecationWarning, stacklevel=2)
+			deprecation_warning(msg)
 			return func(*args, **kwargs)
 
 		return inner
diff --git a/collections_extended/bags.py b/collections_extended/bags.py
index c23094f..fa67503 100644
--- a/collections_extended/bags.py
+++ b/collections_extended/bags.py
@@ -188,7 +188,7 @@ class _basebag(Set):
 	@deprecated(
 		"Use `heapq.nlargest(n, self.counts(), key=itemgetter(1))` instead or "
 		"`sorted(self.counts(), reverse=True, key=itemgetter(1))` for `n=None`",
-		'1.1',
+		'1.0',
 		)
 	def nlargest(self, n=None):
 		"""List the n most common elements and their counts.
diff --git a/collections_extended/indexed_dict.py b/collections_extended/indexed_dict.py
index a1c9275..2b6fc0f 100644
--- a/collections_extended/indexed_dict.py
+++ b/collections_extended/indexed_dict.py
@@ -4,7 +4,7 @@
 """
 import collections
 
-from ._util import NOT_SET
+from ._util import NOT_SET, deprecation_warning
 
 __all__ = ('IndexedDict', )
 
@@ -33,38 +33,82 @@ class IndexedDict(collections.MutableMapping):
 		self._dict = {}
 		self._list = []
 
-	def get(self, key=NOT_SET, index=NOT_SET, d=None):
-		"""Return value with given key or index.
+	def get(self, key=NOT_SET, index=NOT_SET, default=NOT_SET, d=NOT_SET):
+		"""Return value with given `key` or `index`.
 
-		If no value is found, return d (None by default).
+		If no value is found, return `default` (`None` by default).
+
+		.. deprecated :: 1.1
+		The `d` parameter has been renamed `default`. `d` will be removed in
+		some future version.
+
+		Args:
+			key: The key of the value to get
+			index: The index of the value to get
+			default: The value to return if `key` is not found or `index` is
+				out of bounds. If it is NOT_SET, None is returned.
+			d: DEPRECATED: Old parameter name for `default`
 		"""
+		if d is not NOT_SET:
+			if default is not NOT_SET:
+				raise ValueError('Specified default and d')
+			deprecation_warning(
+				"IndexedDict.pop parameter 'd' has been renamed to 'default'"
+				)
+			default = d
+		if default is NOT_SET:
+			default = None
+
 		if index is NOT_SET and key is not NOT_SET:
 			try:
 				index, value = self._dict[key]
 			except KeyError:
-				return d
+				return default
 			else:
 				return value
 		elif index is not NOT_SET and key is NOT_SET:
 			try:
 				key, value = self._list[index]
 			except IndexError:
-				return d
+				return default
 			else:
 				return value
 		else:
 			raise KEY_EQ_INDEX_ERROR
 
-	def pop(self, key=NOT_SET, index=NOT_SET, d=NOT_SET):
-		"""Remove and return value with given key or index (last item by default).
+	def pop(self, key=NOT_SET, index=NOT_SET, default=NOT_SET, d=NOT_SET):
+		"""Remove and return value.
+
+		Optionally, specify the `key` or `index` of the value to pop.
+		If `key` is specified and is not found a `KeyError` is raised unless
+		`default` is specified. Likewise, if `index` is specified that is out of
+		bounds, an `IndexError` is raised unless `default` is specified.
 
-		If key is not found, returns d if given,
-		otherwise raises KeyError or IndexError.
+		Both `index` and `key` cannot be specified. If neither is specified,
+		then the last value is popped.
 
 		This is generally O(N) unless removing last item, then O(1).
-		"""
-		has_default = d is not NOT_SET
 
+		.. deprecated :: 1.1
+		The `d` parameter has been renamed `default`. `d` will be removed in
+		some future version.
+
+		Args:
+			key: The key of the value to pop
+			index: The index of the value to pop
+			default: The value to return if the key is not found or the index is
+				out of bounds
+			d: DEPRECATED: Old parameter name for `default`
+		"""
+		if d is not NOT_SET:
+			if default is not NOT_SET:
+				raise ValueError('Specified default and d')
+			deprecation_warning(
+				"IndexedDict.pop parameter 'd' has been renamed to 'default'"
+				)
+			default = d
+
+		has_default = default is not NOT_SET
 		if index is NOT_SET and key is not NOT_SET:
 			index, value = self._pop_key(key, has_default)
 		elif key is NOT_SET:
@@ -73,7 +117,7 @@ class IndexedDict(collections.MutableMapping):
 			raise KEY_AND_INDEX_ERROR
 
 		if index is None:
-			return d
+			return default
 		else:
 			self._fix_indices_after_delete(index)
 			return value


Test patch:
diff --git a/tests/test_bags.py b/tests/test_bags.py
index 0e66f01..43f9d39 100644
--- a/tests/test_bags.py
+++ b/tests/test_bags.py
@@ -66,10 +66,8 @@ def test_nlargest():
 def test_nlargest_deprecated():
 	"""Test that nlargest raises a DeprecationWarning."""
 	b = bag()
-	with warnings.catch_warnings():
-		warnings.simplefilter('error')
-		with pytest.raises(DeprecationWarning):
-			b.nlargest()
+	with pytest.deprecated_call():
+		b.nlargest()
 
 
 def test_from_map():
diff --git a/tests/test_indexed_dict.py b/tests/test_indexed_dict.py
index af5f81e..b6c1f0e 100644
--- a/tests/test_indexed_dict.py
+++ b/tests/test_indexed_dict.py
@@ -1,6 +1,3 @@
-# pylint: disable=redefined-outer-name
-# pylint: disable=W0212
-
 import pytest
 
 from collections_extended.indexed_dict import IndexedDict
@@ -11,7 +8,7 @@ def assert_internal_state(self):
 	Returns True, so it can be used in an assert expression itself."""
 
 	assert len(self._dict) == len(self._list)
-	for k, (i, v) in self._dict.items():
+	for k, (i, v) in self._dict.items():  # noqa
 		k2, v2 = self._list[i]
 		assert k2 == k
 		assert v2 is v
@@ -61,11 +58,32 @@ def test_get_key_found(d, indexing):
 	assert d.get(**indexing) == 11
 
 
+@pytest.mark.parametrize("indexing", [{"key": "x"}, {"index": 100}, {"index": -6}])
+def test_get_specifying_missing_default(d, indexing):
+	assert d.get(default=5, **indexing) == 5
+
+
+def test_get_deprecated_param(d):
+	with pytest.deprecated_call():
+		assert d.get('x', d='XXX') == 'XXX'
+
+
 @pytest.mark.parametrize("indexing", [{"key": "x"}, {"index": 100}, {"index": -6}])
 def test_get_missing_default(d, indexing):
 	assert d.get(**indexing) is None
 
 
+def test_get_duplicate_default(d):
+	with pytest.raises(ValueError):
+		d.get(d=None, default=None)
+	with pytest.raises(ValueError):
+		d.get(d='XXX', default=None)
+	with pytest.raises(ValueError):
+		d.get(d=None, default='XXX')
+	with pytest.raises(ValueError):
+		d.get(d='XXX', default='XXX')
+
+
 def test_get_both_key_and_index(d):
 	with pytest.raises(TypeError):
 		d.get(key="a", index=4)
@@ -93,6 +111,11 @@ def test_pop_missing_default(d, indexing):
 	assert list(d) == list("abcde")
 
 
+def test_pop_duplicate_default(d):
+	with pytest.raises(ValueError):
+		d.pop(d='XXX', default='XXX')
+
+
 def test_pop_missing_key_no_default(d):
 	with pytest.raises(KeyError):
 		d.pop("X")
@@ -106,6 +129,11 @@ def test_pop_missing_index_no_default(d, index):
 	assert list(d) == list("abcde")
 
 
+def test_deprecated_pop_default(d):
+	with pytest.deprecated_call():
+		assert d.pop(999, d='XXX') == 'XXX'
+
+
 def test_pop_empty_default():
 	d = IndexedDict()
 	assert d.pop(d="XXX") == "XXX"