Merge pull request #158 from hoytak/more-hypervariant-flextype-recording-power

Flex types always converted correctly in cy_variant

Author: Yucheng Low

oss_src/unity/python/sframe/cython/cy_flexible_type.pyx

@@ -1270,7 +1270,7 @@ cdef flexible_type _ft_translate(object v, int tr_code) except *:
 
     # These are optimized by the cython compiler into a big switch statement.
     if tr_code == FT_INT_TYPE:
-        ret.set_int(<long>v)
+        ret.set_int(<flex_int>v)
         return ret
     elif tr_code == FT_FLOAT_TYPE:
         ret.set_double(<double>v)

oss_src/unity/python/sframe/cython/cy_variant.pyx

@@ -22,10 +22,11 @@ from .cy_model cimport UnityModel
 from .cy_sframe cimport UnitySFrameProxy
 from .cy_sarray cimport UnitySArrayProxy
 
-from .cy_flexible_type cimport flexible_type, flex_list, flex_dict
+from .cy_flexible_type cimport flexible_type, flex_list, flex_dict, flex_int
 from .cy_flexible_type cimport flexible_type_from_pyobject
 from .cy_flexible_type cimport pyobject_from_flexible_type
 from .cy_flexible_type cimport check_list_to_vector_translation
+from .cy_flexible_type cimport flex_type_enum, UNDEFINED
 
 from .cy_dataframe cimport gl_dataframe
 from .cy_dataframe cimport is_pandas_dataframe
@@ -53,10 +54,10 @@ from libcpp.map cimport map
 
 # Fast track simple flexible types
 DEF VAR_TR_FT_INT                      = 0
-DEF VAR_TR_FT_LONG                     = 1
-DEF VAR_TR_FT_FLOAT                    = 2
-DEF VAR_TR_FT_STR                      = 3
-DEF VAR_TR_FT_UNICODE                  = 4
+DEF VAR_TR_FT_FLOAT                    = 1
+DEF VAR_TR_FT_STR                      = 2
+DEF VAR_TR_FT_UNICODE                  = 3
+DEF VAR_TR_FT_NONE                     = 4
 
 # Nested types
 DEF VAR_TR_DICT                        = 5
@@ -139,16 +140,18 @@ cdef int _get_tr_code_by_type_string(object v) except -1:
     if not internal_classes_set:
         import_internal_classes()
 
-    if type(v) is int:
+    if type(v) is int or type(v) is long:
         ret =  VAR_TR_FT_INT
-    elif type(v) is long:
-        ret =  VAR_TR_FT_LONG
     elif type(v) is float:
         ret =  VAR_TR_FT_FLOAT
     elif type(v) is str:
         ret =  VAR_TR_FT_STR
     elif type(v) is unicode_type:
         ret =  VAR_TR_FT_UNICODE
+    elif type(v) is bool:
+        ret =  VAR_TR_FT_INT
+    elif v is None:
+        ret =  VAR_TR_FT_NONE
     elif type(v) is list:
         ret =  VAR_TR_LIST
     elif type(v) is tuple:
@@ -306,10 +309,7 @@ cdef bint _var_set_listlike_internal(variant_vector_type& ret_as_vv,
         element_stored_in_flex_list = False
 
         if tr_code == VAR_TR_FT_INT:
-            ret_as_fl[i].set_int(<int>x)
-            element_stored_in_flex_list = True
-        elif tr_code == VAR_TR_FT_LONG:
-            ret_as_fl[i].set_int(<long>x)
+            ret_as_fl[i].set_int(<flex_int>x)
             element_stored_in_flex_list = True
         elif tr_code == VAR_TR_FT_FLOAT:
             ret_as_fl[i].set_double(<double>x)
@@ -320,6 +320,9 @@ cdef bint _var_set_listlike_internal(variant_vector_type& ret_as_vv,
         elif tr_code == VAR_TR_FT_UNICODE:
             ret_as_fl[i].set_string((<unicode>x))
             element_stored_in_flex_list = True
+        elif tr_code == VAR_TR_FT_NONE:
+            ret_as_fl[i] = flexible_type(UNDEFINED)
+            element_stored_in_flex_list = True
         elif tr_code == VAR_TR_LIST or tr_code == VAR_TR_TUPLE:
             ret_as_fl[i].set_list(flex_list())
 
@@ -406,8 +409,10 @@ cdef inline bint _var_set_dict_internal(variant_map_type& ret_as_vm, flex_dict&
 
     cdef long pos
     cdef int tr_code
-    cdef bint writing_to_flex_dict
-    
+    cdef bint writing_to_flex_dict, sub_is_flex_type
+    cdef variant_vector_type sub_vv = variant_vector_type()
+    cdef variant_map_type sub_vm = variant_map_type()
+
     if output_can_be_varmap:
         # First see if it can be translated as flexible type, which
         # would be great.
@@ -426,17 +431,65 @@ cdef inline bint _var_set_dict_internal(variant_map_type& ret_as_vm, flex_dict&
                     tr_code = get_var_tr_code(v)
 
                     if tr_code == VAR_TR_FT_INT:
-                        ret_as_fd[pos].second.set_int(<int>v)
-                    elif tr_code == VAR_TR_FT_LONG:
-                        ret_as_fd[pos].second.set_int(<long>v)
+                        ret_as_fd[pos].second.set_int(<flex_int>v)
                     elif tr_code == VAR_TR_FT_FLOAT:
                         ret_as_fd[pos].second.set_double(<double>v)
                     elif tr_code == VAR_TR_FT_STR:
                         ret_as_fd[pos].second.set_string(<str>v)
                     elif tr_code == VAR_TR_FT_UNICODE:
                         ret_as_fd[pos].second.set_string((<unicode>v))
+                    elif tr_code == VAR_TR_FT_NONE:
+                        ret_as_fd[pos].second = flexible_type(UNDEFINED)
                     elif tr_code == VAR_TR_ATTEMPT_OTHER_FLEXIBLE_TYPE:
                         ret_as_fd[pos].second = _translate_to_flexible_type(v)
+                    elif tr_code == VAR_TR_LIST or tr_code == VAR_TR_TUPLE:
+                        ret_as_fd[pos].second.set_list(flex_list())
+
+                        if tr_code == VAR_TR_LIST:
+                            sub_is_flex_type = _var_set_listlike_internal(
+                                sub_vv, ret_as_fd[pos].second.get_list_m(), <list>v, False)
+                        else:
+                            sub_is_flex_type = _var_set_listlike_internal(
+                                sub_vv, ret_as_fd[pos].second.get_list_m(), <tuple>v, False)
+
+                        if sub_is_flex_type:
+                            check_list_to_vector_translation(ret_as_fd[pos].second)
+
+                            if not writing_to_flex_dict:
+                                variant_set_flexible_type(ret_as_vm[ret_as_fd[pos].first.get_string()], 
+                                                          ret_as_fd[pos].second)
+                        else:                                                            
+                            if writing_to_flex_dict:
+                                ret_as_vm.clear()
+                                for i in range(pos):
+                                    variant_set_flexible_type(ret_as_vm[ret_as_fd[i].first.get_string()],
+                                                              ret_as_fd[i].second)
+
+                                writing_to_flex_dict = False
+
+                                
+                            variant_set_variant_vector(ret_as_vm[ret_as_fd[pos].first.get_string()], sub_vv)
+
+                    elif tr_code == VAR_TR_DICT:
+                        ret_as_fd[pos].second.set_dict(flex_dict())
+
+                        sub_is_flex_type = _var_set_dict_internal(sub_vm, ret_as_fd[pos].second.get_dict_m(), <dict>v, False)
+
+                        if sub_is_flex_type:
+                            if not writing_to_flex_dict:
+                                variant_set_flexible_type(ret_as_vm[ret_as_fd[pos].first.get_string()], 
+                                                          ret_as_fd[pos].second)
+                        else:
+                            if writing_to_flex_dict:
+                                ret_as_vm.clear()
+                                for i in range(pos):
+                                    variant_set_flexible_type(ret_as_vm[ret_as_fd[i].first.get_string()],
+                                                              ret_as_fd[i].second)
+
+                                writing_to_flex_dict = False
+
+                            variant_set_variant_map(ret_as_vm[ret_as_fd[pos].first.get_string()], sub_vm)
+                        
                     else:                    
                         # That didn't work -- translate the rest as
                         # variant type if it can be a varmap.
@@ -555,20 +608,19 @@ cdef _convert_to_variant_type(variant_type& ret, object v, int tr_code):
 
     # Fast-tracked flexible type versions
     if tr_code == VAR_TR_FT_INT:
-        ft.set_int(<int>v)
-        variant_set_flexible_type(ret, ft)
-    elif tr_code == VAR_TR_FT_LONG:
-        ft.set_int(<long>v)
+        ft.set_int(<flex_int>v)
         variant_set_flexible_type(ret, ft)
     elif tr_code == VAR_TR_FT_FLOAT:
-        ft.set_double(<float>v)
+        ft.set_double(<double>v)
         variant_set_flexible_type(ret, ft)
     elif tr_code == VAR_TR_FT_STR:
         ft.set_string(<str>v)
         variant_set_flexible_type(ret, ft)
     elif tr_code == VAR_TR_FT_UNICODE:
         ft.set_string(<unicode>v)
         variant_set_flexible_type(ret, ft)
+    elif tr_code == VAR_TR_FT_NONE:
+        variant_set_flexible_type(ret, flexible_type(UNDEFINED))
 
     # Nested container types
     elif tr_code == VAR_TR_DICT:
@@ -611,17 +663,16 @@ cdef _convert_to_variant_type(variant_type& ret, object v, int tr_code):
         variant_set_closure(ret, make_function_closure_info(v))
 
     # Flexible type -- this is actually the last resort since we
-    # assume the aboves are exact matches and do not include the
-    # flexible type stuff.
+    # assume the above types are exact matches and do not include the
+    # flexible type stuff, save for the fast paths for common types.
     elif tr_code == VAR_TR_ATTEMPT_OTHER_FLEXIBLE_TYPE:
         try:
             variant_set_flexible_type(ret, flexible_type_from_pyobject(v))
         except TypeError:
             raise_translation_error(v)
 
     else:
-        assert False
-
+        assert False    
 
 ################################################################################
 # The main translation function
@@ -693,3 +744,14 @@ cdef to_value(PyCommClient cli, variant_type& v):
         return to_vector(cli, variant_get_variant_vector(v))
     else:
         raise TypeError("Unsupported variant type.")
+
+################################################################################
+# Routines to assist with debugging. 
+    
+def _debug_is_flexible_type_encoded(object obj):
+    """
+    Checks to make sure that if an object can be encoded as a flexible
+    type, then it is.
+    """
+    cdef variant_type vt = from_value(obj)
+    return (vt.which() == VAR_TYPE_FLEXIBLE_TYPE)            

oss_src/unity/python/sframe/test/test_extensions.py

@@ -10,6 +10,8 @@
 from ..data_structures.sframe import SFrame
 import random
 
+from ..cython.cy_variant import _debug_is_flexible_type_encoded
+
 
 class VariantCheckTest(unittest.TestCase):
 
@@ -67,6 +69,10 @@ def test_stress(self):
 
         random.seed(0)
 
+
+        class A: pass
+        A.flextype_encodable = True
+
         def _make(depth):
 
             if depth == 0:
@@ -79,8 +85,10 @@ def _make(depth):
             elif s == 1:
                 return random.randint(0,100000)
             elif s == 2:
+                A.flextype_encodable = False
                 return SArray([random.randint(0,100000) for i in range(2)])
             elif s == 3:
+                A.flextype_encodable = False
                 return SFrame({'a' : [random.randint(0,100000) for i in range(2)],
                                'b' : [str(random.randint(0,100000)) for i in range(2)]})
 
@@ -93,5 +101,18 @@ def _make(depth):
                 return {str(random.randint(0, 100)) : _make(depth - 1)
                         for i in range(length)}
 
-        for i in range(10):
-            self.variant_turnaround(_make(5 + i))
+        for depth in [2,3,4,5,10]:
+            for i in range(10):
+                
+                A.flextype_encodable = True
+
+                obj = _make(depth)
+
+                # Test that it's losslessly encoded and decoded
+                self.variant_turnaround(obj)
+
+                # Test that if it can be 
+                if _debug_is_flexible_type_encoded(obj):
+                    self.assertTrue(A.flextype_encodable)
+                else:
+                    self.assertFalse(A.flextype_encodable)