force mapping to be a single dict object on MSET, MSETNX and ZADD

Previously MSET, MSETNX and ZADD accepted multiple ways to specify the
mapping of keys to values including via **kwargs. This turned out to be
a poor choice. As Redis evolved and added additional options to the ZADD
command, these options couldn't be specified in redis-py without possible
element name conflictd. This fixes that going forward and makes the commands
simpler.

2 files changed, 17 insertions, 47 deletions

diff --git a/redis/client.py b/redis/client.py
index 94853b6..05e8b0d 100755
--- a/redis/client.py
+++ b/redis/client.py
@@ -1249,33 +1249,26 @@ class StrictRedis(object):
             options[EMPTY_RESPONSE] = []
         return self.execute_command('MGET', *args, **options)
 
-    def mset(self, *args, **kwargs):
+    def mset(self, mapping):
         """
-        Sets key/values based on a mapping. Mapping can be supplied as a single
-        dictionary argument or as kwargs.
+        Sets key/values based on a mapping. Mapping is a dictionary of
+        key/value pairs. Both keys and values should be strings or types that
+        can be cast to a string via str().
         """
-        if args:
-            if len(args) != 1 or not isinstance(args[0], dict):
-                raise RedisError('MSET requires **kwargs or a single dict arg')
-            kwargs.update(args[0])
         items = []
-        for pair in iteritems(kwargs):
+        for pair in iteritems(mapping):
             items.extend(pair)
         return self.execute_command('MSET', *items)
 
-    def msetnx(self, *args, **kwargs):
+    def msetnx(self, mapping):
         """
         Sets key/values based on a mapping if none of the keys are already set.
-        Mapping can be supplied as a single dictionary argument or as kwargs.
+        Mapping is a dictionary of key/value pairs. Both keys and values
+        should be strings or types that can be cast to a string via str().
         Returns a boolean indicating if the operation was successful.
         """
-        if args:
-            if len(args) != 1 or not isinstance(args[0], dict):
-                raise RedisError('MSETNX requires **kwargs or a single '
-                                 'dict arg')
-            kwargs.update(args[0])
         items = []
-        for pair in iteritems(kwargs):
+        for pair in iteritems(mapping):
             items.extend(pair)
         return self.execute_command('MSETNX', *items)
 
@@ -2209,24 +2202,15 @@ class StrictRedis(object):
         return self.execute_command('XTRIM', name, *pieces)
 
     # SORTED SET COMMANDS
-    def zadd(self, name, *args, **kwargs):
+    def zadd(self, name, mapping):
         """
-        Set any number of score, element-name pairs to the key ``name``. Pairs
-        can be specified in two ways:
-
-        As *args, in the form of: score1, name1, score2, name2, ...
-        or as **kwargs, in the form of: name1=score1, name2=score2, ...
-
-        The following example would add four values to the 'my-key' key:
-        redis.zadd('my-key', 1.1, 'name1', 2.2, 'name2', name3=3.3, name4=4.4)
+        Set any number of element-name, score pairs to the key ``name``. Pairs
+        are specified as a dict of element-names keys to score values.
         """
+        if not mapping:
+            raise RedisError("ZADD requires at least one element/score pair")
         pieces = []
-        if args:
-            if len(args) % 2 != 0:
-                raise RedisError("ZADD requires an equal number of "
-                                 "values and scores")
-            pieces.extend(args)
-        for pair in iteritems(kwargs):
+        for pair in iteritems(mapping):
             pieces.append(pair[1])
             pieces.append(pair[0])
         return self.execute_command('ZADD', name, *pieces)
diff --git a/tests/test_commands.py b/tests/test_commands.py
index a264d3a..de94a71 100644
--- a/tests/test_commands.py
+++ b/tests/test_commands.py
@@ -481,12 +481,6 @@ class TestRedisCommands(object):
         for k, v in iteritems(d):
             assert r[k] == v
 
-    def test_mset_kwargs(self, r):
-        d = {'a': b'1', 'b': b'2', 'c': b'3'}
-        assert r.mset(**d)
-        for k, v in iteritems(d):
-            assert r[k] == v
-
     def test_msetnx(self, r):
         d = {'a': b'1', 'b': b'2', 'c': b'3'}
         assert r.msetnx(d)
@@ -496,15 +490,6 @@ class TestRedisCommands(object):
             assert r[k] == v
         assert r.get('d') is None
 
-    def test_msetnx_kwargs(self, r):
-        d = {'a': b'1', 'b': b'2', 'c': b'3'}
-        assert r.msetnx(**d)
-        d2 = {'a': b'x', 'd': b'4'}
-        assert not r.msetnx(**d2)
-        for k, v in iteritems(d):
-            assert r[k] == v
-        assert r.get('d') is None
-
     @skip_if_server_version_lt('2.6.0')
     def test_pexpire(self, r):
         assert not r.pexpire('a', 60000)
@@ -2200,7 +2185,8 @@ class TestRedisCommands(object):
 
 class TestStrictCommands(object):
     def test_strict_zadd(self, sr):
-        sr.zadd('a', 1.0, 'a1', 2.0, 'a2', a3=3.0)
+        mapping = {'a1': 1.0, 'a2': 2.0, 'a3': 3.0}
+        sr.zadd('a', mapping)
         assert sr.zrange('a', 0, -1, withscores=True) == \
             [(b'a1', 1.0), (b'a2', 2.0), (b'a3', 3.0)]