Repository: maximkulkin/lollipop

Issue:
Misleading Tuple type
Is it on purpose that the Tuple type dumps into / loads from a list instead of a tuple? If yes, I think the naming is a bit misleading…

```
Tuple([String(), Integer(), Boolean()]).load(('foo', 123, False))
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "site-packages\lollipop\types.py", line 443, in load self._fail('invalid')
  File "site-packages\lollipop\errors.py", line 63, in _fail raise ValidationError(msg)
lollipop.errors.ValidationError: Invalid data: 'Value should be list'
```

Discussion:

Patch:
diff --git a/lollipop/types.py b/lollipop/types.py
index 5f3ac70..3ec8a50 100644
--- a/lollipop/types.py
+++ b/lollipop/types.py
@@ -1,6 +1,6 @@
 from lollipop.errors import ValidationError, ValidationErrorBuilder, \
     ErrorMessagesMixin, merge_errors
-from lollipop.utils import is_list, is_dict, make_context_aware, \
+from lollipop.utils import is_sequence, is_mapping, make_context_aware, \
     constant, identity, OpenStruct
 from lollipop.compat import string_types, int_types, iteritems, OrderedDict
 import datetime
@@ -377,7 +377,7 @@ class List(Type):
             self._fail('required')
 
         # TODO: Make more intelligent check for collections
-        if not is_list(data):
+        if not is_sequence(data):
             self._fail('invalid')
 
         errors_builder = ValidationErrorBuilder()
@@ -395,7 +395,7 @@ class List(Type):
         if value is MISSING or value is None:
             self._fail('required')
 
-        if not is_list(value):
+        if not is_sequence(value):
             self._fail('invalid')
 
         errors_builder = ValidationErrorBuilder()
@@ -422,6 +422,7 @@ class Tuple(Type):
     Example: ::
 
         Tuple([String(), Integer(), Boolean()]).load(['foo', 123, False])
+        # => ('foo', 123, False)
 
     :param list item_types: List of item types.
     :param kwargs: Same keyword arguments as for :class:`Type`.
@@ -439,11 +440,13 @@ class Tuple(Type):
         if data is MISSING or data is None:
             self._fail('required')
 
-        if not is_list(data):
+        if not is_sequence(data):
             self._fail('invalid')
 
         if len(data) != len(self.item_types):
-            self._fail('invalid_length', expected_length=len(self.item_types))
+            self._fail('invalid_length',
+                       expected_length=len(self.item_types),
+                       actual_length=len(data))
 
         errors_builder = ValidationErrorBuilder()
         result = []
@@ -454,13 +457,13 @@ class Tuple(Type):
                 errors_builder.add_errors({idx: ve.messages})
         errors_builder.raise_errors()
 
-        return super(Tuple, self).load(result, *args, **kwargs)
+        return tuple(super(Tuple, self).load(result, *args, **kwargs))
 
     def dump(self, value, *args, **kwargs):
         if value is MISSING or value is None:
             self._fail('required')
 
-        if not is_list(value):
+        if not is_sequence(value):
             self._fail('invalid')
 
         if len(value) != len(self.item_types):
@@ -560,7 +563,7 @@ class OneOf(Type):
         if data is MISSING or data is None:
             self._fail('required')
 
-        if is_dict(self.types) and self.load_hint:
+        if is_mapping(self.types) and self.load_hint:
             type_id = self.load_hint(data)
             if type_id not in self.types:
                 self._fail('unknown_type_id', type_id=type_id)
@@ -569,7 +572,8 @@ class OneOf(Type):
             result = item_type.load(data, *args, **kwargs)
             return super(OneOf, self).load(result, *args, **kwargs)
         else:
-            for item_type in (self.types.values() if is_dict(self.types) else self.types):
+            for item_type in (self.types.values()
+                              if is_mapping(self.types) else self.types):
                 try:
                     result = item_type.load(data, *args, **kwargs)
                     return super(OneOf, self).load(result, *args, **kwargs)
@@ -582,7 +586,7 @@ class OneOf(Type):
         if data is MISSING or data is None:
             self._fail('required')
 
-        if is_dict(self.types) and self.dump_hint:
+        if is_mapping(self.types) and self.dump_hint:
             type_id = self.dump_hint(data)
             if type_id not in self.types:
                 self._fail('unknown_type_id', type_id=type_id)
@@ -591,7 +595,8 @@ class OneOf(Type):
             result = item_type.dump(data, *args, **kwargs)
             return super(OneOf, self).dump(result, *args, **kwargs)
         else:
-            for item_type in (self.types.values() if is_dict(self.types) else self.types):
+            for item_type in (self.types.values()
+                              if is_mapping(self.types) else self.types):
                 try:
                     result = item_type.dump(data, *args, **kwargs)
                     return super(OneOf, self).dump(result, *args, **kwargs)
@@ -668,7 +673,7 @@ class Dict(Type):
         if data is MISSING or data is None:
             self._fail('required')
 
-        if not is_dict(data):
+        if not is_mapping(data):
             self._fail('invalid')
 
         errors_builder = ValidationErrorBuilder()
@@ -695,7 +700,7 @@ class Dict(Type):
         if value is MISSING or value is None:
             self._fail('required')
 
-        if not is_dict(value):
+        if not is_mapping(value):
             self._fail('invalid')
 
         errors_builder = ValidationErrorBuilder()
@@ -1091,10 +1096,10 @@ class Object(Type):
 
         if isinstance(bases_or_fields, Type):
             bases = [bases_or_fields]
-        if is_list(bases_or_fields) and \
+        if is_sequence(bases_or_fields) and \
                 all([isinstance(base, Type) for base in bases_or_fields]):
             bases = bases_or_fields
-        elif is_list(bases_or_fields) or is_dict(bases_or_fields):
+        elif is_sequence(bases_or_fields) or is_mapping(bases_or_fields):
             if fields is None:
                 bases = []
                 fields = bases_or_fields
@@ -1108,9 +1113,9 @@ class Object(Type):
         self._allow_extra_fields = allow_extra_fields
         self._immutable = immutable
         self._ordered = ordered
-        if only is not None and not is_list(only):
+        if only is not None and not is_sequence(only):
             only = [only]
-        if exclude is not None and not is_list(exclude):
+        if exclude is not None and not is_sequence(exclude):
             exclude = [exclude]
         self._only = only
         self._exclude = exclude
@@ -1155,7 +1160,8 @@ class Object(Type):
         if fields is not None:
             all_fields += [
                 (name, self._normalize_field(field))
-                for name, field in (iteritems(fields) if is_dict(fields) else fields)
+                for name, field in (iteritems(fields)
+                                    if is_mapping(fields) else fields)
             ]
 
         return OrderedDict(all_fields)
@@ -1164,7 +1170,7 @@ class Object(Type):
         if data is MISSING or data is None:
             self._fail('required')
 
-        if not is_dict(data):
+        if not is_mapping(data):
             self._fail('invalid')
 
         errors_builder = ValidationErrorBuilder()
@@ -1213,7 +1219,7 @@ class Object(Type):
         if data is None:
             self._fail('required')
 
-        if not is_dict(data):
+        if not is_mapping(data):
             self._fail('invalid')
 
         errors_builder = ValidationErrorBuilder()
@@ -1528,7 +1534,7 @@ def validated_type(base_type, name=None, validate=None):
     """
     if validate is None:
         validate = []
-    if not is_list(validate):
+    if not is_sequence(validate):
         validate = [validate]
 
     class ValidatedSubtype(base_type):
diff --git a/lollipop/utils.py b/lollipop/utils.py
index 596706c..fa0bb4b 100644
--- a/lollipop/utils.py
+++ b/lollipop/utils.py
@@ -1,6 +1,7 @@
 import inspect
 import re
 from lollipop.compat import DictMixin, iterkeys
+import collections
 
 
 def identity(value):
@@ -14,14 +15,18 @@ def constant(value):
     return func
 
 
-def is_list(value):
+def is_sequence(value):
     """Returns True if value supports list interface; False - otherwise"""
-    return isinstance(value, list)
+    return isinstance(value, collections.Sequence)
 
-
-def is_dict(value):
+def is_mapping(value):
     """Returns True if value supports dict interface; False - otherwise"""
-    return isinstance(value, dict)
+    return isinstance(value, collections.Mapping)
+
+
+# Backward compatibility
+is_list = is_sequence
+is_dict = is_mapping
 
 
 def make_context_aware(func, numargs):
diff --git a/lollipop/validators.py b/lollipop/validators.py
index 8652d96..c29b1da 100644
--- a/lollipop/validators.py
+++ b/lollipop/validators.py
@@ -1,7 +1,7 @@
 from lollipop.errors import ValidationError, ValidationErrorBuilder, \
     ErrorMessagesMixin
 from lollipop.compat import string_types, iteritems
-from lollipop.utils import make_context_aware, is_list, identity
+from lollipop.utils import make_context_aware, is_sequence, identity
 import re
 
 
@@ -291,7 +291,7 @@ class Unique(Validator):
             self._error_messages['unique'] = error
 
     def __call__(self, value, context=None):
-        if not is_list(value):
+        if not is_sequence(value):
             self._fail('invalid')
 
         seen = set()
@@ -318,12 +318,12 @@ class Each(Validator):
 
     def __init__(self, validators, **kwargs):
         super(Validator, self).__init__(**kwargs)
-        if not is_list(validators):
+        if not is_sequence(validators):
             validators = [validators]
         self.validators = validators
 
     def __call__(self, value, context=None):
-        if not is_list(value):
+        if not is_sequence(value):
             self._fail('invalid', data=value)
 
         error_builder = ValidationErrorBuilder()


Test patch:
diff --git a/tests/test_types.py b/tests/test_types.py
index 2ab07e1..e652fb6 100644
--- a/tests/test_types.py
+++ b/tests/test_types.py
@@ -509,7 +509,7 @@ class TestList(NameDescriptionTestsMixin, RequiredTestsMixin, ValidationTestsMix
 
     def test_loading_non_list_value_raises_ValidationError(self):
         with pytest.raises(ValidationError) as exc_info:
-            List(String()).load('1, 2, 3')
+            List(String()).load(123)
         assert exc_info.value.messages == List.default_error_messages['invalid']
 
     def test_loading_list_value_with_items_of_incorrect_type_raises_ValidationError(self):
@@ -542,9 +542,13 @@ class TestList(NameDescriptionTestsMixin, RequiredTestsMixin, ValidationTestsMix
     def test_dumping_list_value(self):
         assert List(String()).dump(['foo', 'bar', 'baz']) == ['foo', 'bar', 'baz']
 
+    def test_dumping_sequence_value(self):
+        assert List(String()).dump(('foo', 'bar', 'baz')) == ['foo', 'bar', 'baz']
+        assert List(String()).dump('foobar') == ['f', 'o', 'o', 'b', 'a', 'r']
+
     def test_dumping_non_list_value_raises_ValidationError(self):
         with pytest.raises(ValidationError) as exc_info:
-            List(String()).dump('1, 2, 3')
+            List(String()).dump(123)
         assert exc_info.value.messages == List.default_error_messages['invalid']
 
     def test_dumping_list_value_with_items_of_incorrect_type_raises_ValidationError(self):
@@ -563,15 +567,15 @@ class TestList(NameDescriptionTestsMixin, RequiredTestsMixin, ValidationTestsMix
 class TestTuple(NameDescriptionTestsMixin, RequiredTestsMixin, ValidationTestsMixin):
     tested_type = partial(Tuple, [Integer(), Integer()])
     valid_data = [123, 456]
-    valid_value = [123, 456]
+    valid_value = (123, 456)
 
     def test_loading_tuple_with_values_of_same_type(self):
         assert Tuple([Integer(), Integer()]).load([123, 456]) == \
-            [123, 456]
+            (123, 456)
 
     def test_loading_tuple_with_values_of_different_type(self):
         assert Tuple([String(), Integer(), Boolean()]).load(['foo', 123, False]) == \
-            ['foo', 123, False]
+            ('foo', 123, False)
 
     def test_loading_non_tuple_value_raises_ValidationError(self):
         with pytest.raises(ValidationError) as exc_info:
@@ -596,23 +600,35 @@ class TestTuple(NameDescriptionTestsMixin, RequiredTestsMixin, ValidationTestsMi
         assert inner_type.load_context == context
 
     def test_dump_tuple(self):
-        assert Tuple([Integer(), Integer()]).dump([123, 456]) == [123, 456]
+        assert Tuple([String(), Integer()]).dump(('hello', 123)) == ['hello', 123]
+
+    def test_dump_sequence(self):
+        assert Tuple([String(), Integer()]).dump(['hello', 123]) == ['hello', 123]
 
     def test_dumping_non_tuple_raises_ValidationError(self):
         with pytest.raises(ValidationError) as exc_info:
-            Tuple(String()).dump('foo')
+            Tuple([String()]).dump(123)
         assert exc_info.value.messages == Tuple.default_error_messages['invalid']
 
+    def test_dumping_sequence_of_incorrect_length_raises_ValidationError(self):
+        with pytest.raises(ValidationError) as exc_info:
+            Tuple([String(), Integer()]).dump(['hello', 123, 456])
+        assert exc_info.value.messages == \
+            Tuple.default_error_messages['invalid_length'].format(
+                expected_length=2,
+                actual_length=3,
+            )
+
     def test_dumping_tuple_with_items_of_incorrect_type_raises_ValidationError(self):
         with pytest.raises(ValidationError) as exc_info:
-            Tuple([String(), String()]).dump([123, 456])
+            Tuple([String(), String()]).dump(('hello', 456))
         message = String.default_error_messages['invalid']
-        assert exc_info.value.messages == {0: message, 1: message}
+        assert exc_info.value.messages == {1: message}
 
     def test_dumping_tuple_passes_context_to_inner_type_dump(self):
         inner_type = SpyType()
         context = object()
-        Tuple([inner_type, inner_type]).dump(['foo','foo'], context)
+        Tuple([inner_type, inner_type]).dump(('foo','foo'), context)
         assert inner_type.dump_context == context
 
 
diff --git a/tests/test_validators.py b/tests/test_validators.py
index fa84610..5d6421a 100644
--- a/tests/test_validators.py
+++ b/tests/test_validators.py
@@ -318,7 +318,7 @@ class TestRegexp:
 class TestUnique:
     def test_raising_ValidationError_if_value_is_not_collection(self):
         with raises(ValidationError) as exc_info:
-            Unique()('foo')
+            Unique()(123)
         assert exc_info.value.messages == Unique.default_error_messages['invalid']
 
     def test_matching_empty_collection(self):
@@ -371,7 +371,7 @@ is_small = Predicate(lambda x: x <= 5, 'Value should be small')
 class TestEach:
     def test_raising_ValidationError_if_value_is_not_collection(self):
         with raises(ValidationError) as exc_info:
-            Each(lambda x: x)('foo')
+            Each(lambda x: x)(123)
         assert exc_info.value.messages == Each.default_error_messages['invalid']
 
     def test_matching_empty_collections(self):