43 fix analytic sum operation returns number when only integers parti…

…cipate (#50) * Fixed Analytic returns Number data_type in SUM operation when only Integers participate. Changed some tests output dataStructure. * Refactored type changing to work with unary_implicit_promotion. Now it works with MAX and MIN operations. * Bypassed C901 error (complexity) on Analytic validate. * Aggregation and Analytic operations return Integers when the input its only Integers. Updated some tests. Fixed mypy and ruff errors. * Fixed some tests. return_type and type_to_check are no more changed on execution. * Fixed mypy errors. --------- Signed-off-by: Francisco Javier Hernández del Caño <javier.hernandez@meaningfuldata.eu> Co-authored-by: Francisco Javier Hernández del Caño <javier.hernandez@meaningfuldata.eu>
Meaningful-Data · Jan 16, 2025 · 167c402 · 167c402
1 parent a4259ae
commit 167c402
Show file tree

Hide file tree

Showing 34 changed files with 79 additions and 52 deletions.
diff --git a/src/vtlengine/Operators/Analytic.py b/src/vtlengine/Operators/Analytic.py
@@ -38,6 +38,7 @@
 from vtlengine.Exceptions import SemanticError
 from vtlengine.Model import Component, Dataset, Role
 
+return_integer_operators = [MAX, MIN, SUM]
 
 # noinspection PyMethodOverriding
 class Analytic(Operator.Unary):
@@ -52,10 +53,11 @@ class Analytic(Operator.Unary):
         Evaluate: Ensures the type of data is the correct one to perform the Analytic operators.
     """
 
+    return_integer = None
     sql_op: Optional[str] = None
 
     @classmethod
-    def validate(  # type: ignore[override]
+    def validate(  # type: ignore[override]  # noqa: C901
         cls,
         operand: Dataset,
         partitioning: List[str],
@@ -96,7 +98,11 @@ def validate(  # type: ignore[override]
                 unary_implicit_promotion(
                     operand.components[component_name].data_type, cls.type_to_check
                 )
-            if cls.return_type is not None:
+
+            if cls.op in return_integer_operators:
+                cls.return_integer = isinstance(cls.return_type, Integer)
+
+            elif cls.return_type is not None:
                 result_components[component_name] = Component(
                     name=component_name,
                     data_type=cls.return_type,
@@ -117,14 +123,28 @@ def validate(  # type: ignore[override]
             measures = operand.get_measures()
             if len(measures) == 0:
                 raise SemanticError("1-1-1-8", op=cls.op, name=operand.name)
+
+            if cls.op in return_integer_operators:
+                isNumber = False
+                for measure in measures:
+                    isNumber |= isinstance(measure.data_type, Number)
+                cls.return_integer = not isNumber
+
             if cls.type_to_check is not None:
                 for measure in measures:
                     unary_implicit_promotion(measure.data_type, cls.type_to_check)
-            if cls.return_type is not None:
+
+            if cls.op in return_integer_operators:
+                for measure in measures:
+                    new_measure = copy(measure)
+                    new_measure.data_type = Integer if cls.return_integer else Number
+                    result_components[measure.name] = new_measure
+            elif cls.return_type is not None:
                 for measure in measures:
                     new_measure = copy(measure)
                     new_measure.data_type = cls.return_type
                     result_components[measure.name] = new_measure
+
             if cls.op == COUNT and len(measures) <= 1:
                 measure_name = COMP_NAME_MAPPING[cls.return_type]
                 nullable = False if len(measures) == 0 else measures[0].nullable
@@ -210,6 +230,8 @@ def analyticfunc(
                 measure_query = f"{cls.sql_op}({measure})"
             if cls.op == COUNT and len(measure_names) == 1:
                 measure_query += f" {analytic_str} as {COMP_NAME_MAPPING[cls.return_type]}"
+            elif cls.op in return_integer_operators and cls.return_integer:
+                measure_query = f"CAST({measure_query} {analytic_str} AS INTEGER) as {measure}"
             else:
                 measure_query += f" {analytic_str} as {measure}"
             measure_queries.append(measure_query)
@@ -256,6 +278,10 @@ def evaluate(  # type: ignore[override]
             window=window,
             params=params,
         )
+
+        # if cls.return_type == Integer:
+        #     result.data[measure_names] = result.data[measure_names].astype('Int64')
+
         return result
 
 
@@ -266,6 +292,7 @@ class Max(Analytic):
 
     op = MAX
     sql_op = "MAX"
+    return_integer = False
 
 
 class Min(Analytic):
@@ -275,6 +302,7 @@ class Min(Analytic):
 
     op = MIN
     sql_op = "MIN"
+    return_integer = False
 
 
 class Sum(Analytic):
@@ -283,9 +311,8 @@ class Sum(Analytic):
     """
 
     op = SUM
-    type_to_check = Number
-    return_type = Number
     sql_op = "SUM"
+    return_integer = False
 
 
 class Count(Analytic):

diff --git a/src/vtlengine/Operators/Conditional.py b/src/vtlengine/Operators/Conditional.py
@@ -307,8 +307,8 @@ def evaluate(
     ) -> Union[Scalar, DataComponent, Dataset]:
         result = cls.validate(conditions, thenOps, elseOp)
         for condition in conditions:
-            if isinstance(condition, (DataComponent, Dataset)):
-                condition.data.fillna(False, inplace=True)  # type: ignore[union-attr]
+            if isinstance(condition, (DataComponent, Dataset)) and condition.data is not None:
+                condition.data.fillna(False, inplace=True)
             elif isinstance(condition, Scalar) and condition.value is None:
                 condition.value = False
 

diff --git a/tests/Aggregate/data/DataStructure/input/1-1-1-10-1.json b/tests/Aggregate/data/DataStructure/input/1-1-1-10-1.json
@@ -24,13 +24,13 @@
         {
           "name": "Me_1",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         },
         {
           "name": "Me_2",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Aggregate/data/DataStructure/input/1-1-1-10-2.json b/tests/Aggregate/data/DataStructure/input/1-1-1-10-2.json
@@ -24,13 +24,13 @@
         {
           "name": "Me_1",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         },
         {
           "name": "Me_2",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Aggregate/data/DataStructure/input/1-1-1-15-1.json b/tests/Aggregate/data/DataStructure/input/1-1-1-15-1.json
@@ -24,7 +24,7 @@
         {
           "name": "Me_1",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": false
         }
       ]

diff --git a/tests/Aggregate/data/DataStructure/input/1-1-1-15-2.json b/tests/Aggregate/data/DataStructure/input/1-1-1-15-2.json
@@ -24,7 +24,7 @@
         {
           "name": "Me_1",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": false
         }
       ]

diff --git a/tests/Aggregate/data/DataStructure/input/1-1-1-15-3.json b/tests/Aggregate/data/DataStructure/input/1-1-1-15-3.json
@@ -24,7 +24,7 @@
         {
           "name": "Me_1",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": false
         }
       ]

diff --git a/tests/Aggregate/data/DataStructure/input/1-1-1-16-1.json b/tests/Aggregate/data/DataStructure/input/1-1-1-16-1.json
@@ -24,7 +24,7 @@
         {
           "name": "Me_1",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": false
         }
       ]

diff --git a/tests/Aggregate/data/DataStructure/input/1-1-1-16-2.json b/tests/Aggregate/data/DataStructure/input/1-1-1-16-2.json
@@ -24,7 +24,7 @@
         {
           "name": "Me_1",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": false
         }
       ]

diff --git a/tests/Aggregate/data/DataStructure/input/1-1-1-16-3.json b/tests/Aggregate/data/DataStructure/input/1-1-1-16-3.json
@@ -24,7 +24,7 @@
         {
           "name": "Me_1",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": false
         }
       ]

diff --git a/tests/Aggregate/data/DataStructure/output/1-1-1-10-1.json b/tests/Aggregate/data/DataStructure/output/1-1-1-10-1.json
@@ -6,7 +6,7 @@
         {
           "name": "Me_1",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         },
         {

diff --git a/tests/Aggregate/data/DataStructure/output/1-1-1-15-1.json b/tests/Aggregate/data/DataStructure/output/1-1-1-15-1.json
@@ -12,7 +12,7 @@
         {
           "name": "Me_1",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": false
         }
       ]

diff --git a/tests/Aggregate/data/DataStructure/output/1-1-1-16-1.json b/tests/Aggregate/data/DataStructure/output/1-1-1-16-1.json
@@ -12,7 +12,7 @@
         {
           "name": "Me_1",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": false
         }
       ]

diff --git a/tests/Analytic/data/DataStructure/input/2-1-1-18-1.json b/tests/Analytic/data/DataStructure/input/2-1-1-18-1.json
@@ -54,7 +54,7 @@
         {
           "name": "THRD_PRTY_PRRTY_CLMS",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Analytic/data/DataStructure/input/2-1-1-19-1.json b/tests/Analytic/data/DataStructure/input/2-1-1-19-1.json
@@ -54,7 +54,7 @@
         {
           "name": "THRD_PRTY_PRRTY_CLMS",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Analytic/data/DataStructure/input/2-1-1-20-1.json b/tests/Analytic/data/DataStructure/input/2-1-1-20-1.json
@@ -54,7 +54,7 @@
         {
           "name": "THRD_PRTY_PRRTY_CLMS",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Analytic/data/DataStructure/input/2-1-1-21-1.json b/tests/Analytic/data/DataStructure/input/2-1-1-21-1.json
@@ -54,7 +54,7 @@
         {
           "name": "THRD_PRTY_PRRTY_CLMS",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Analytic/data/DataStructure/input/2-1-1-22-1.json b/tests/Analytic/data/DataStructure/input/2-1-1-22-1.json
@@ -54,7 +54,7 @@
         {
           "name": "THRD_PRTY_PRRTY_CLMS",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Analytic/data/DataStructure/input/2-1-1-23-1.json b/tests/Analytic/data/DataStructure/input/2-1-1-23-1.json
@@ -54,7 +54,7 @@
         {
           "name": "THRD_PRTY_PRRTY_CLMS",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Analytic/data/DataStructure/input/2-1-1-24-1.json b/tests/Analytic/data/DataStructure/input/2-1-1-24-1.json
@@ -54,7 +54,7 @@
         {
           "name": "THRD_PRTY_PRRTY_CLMS",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Analytic/data/DataStructure/input/2-1-1-25-1.json b/tests/Analytic/data/DataStructure/input/2-1-1-25-1.json
@@ -54,7 +54,7 @@
         {
           "name": "THRD_PRTY_PRRTY_CLMS",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Analytic/data/DataStructure/input/2-1-1-26-1.json b/tests/Analytic/data/DataStructure/input/2-1-1-26-1.json
@@ -54,7 +54,7 @@
         {
           "name": "THRD_PRTY_PRRTY_CLMS",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Analytic/data/DataStructure/output/1-1-1-12-1.json b/tests/Analytic/data/DataStructure/output/1-1-1-12-1.json
@@ -24,13 +24,13 @@
         {
           "name": "Me_1",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         },
         {
           "name": "Me_2",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Analytic/data/DataStructure/output/2-1-1-12-1.json b/tests/Analytic/data/DataStructure/output/2-1-1-12-1.json
@@ -36,13 +36,13 @@
         {
           "name": "Me_11",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         },
         {
           "name": "Me_22",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         }
       ]

diff --git a/tests/Analytic/data/DataStructure/output/2-1-1-18-1.json b/tests/Analytic/data/DataStructure/output/2-1-1-18-1.json
@@ -54,13 +54,13 @@
         {
           "name": "THRD_PRTY_PRRTY_CLMS",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         },
         {
           "name": "MX_3PPC",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         },
         {

diff --git a/tests/Analytic/data/DataStructure/output/2-1-1-19-1.json b/tests/Analytic/data/DataStructure/output/2-1-1-19-1.json
@@ -54,13 +54,13 @@
         {
           "name": "THRD_PRTY_PRRTY_CLMS",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         },
         {
           "name": "MX_3PPC",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         },
         {

diff --git a/tests/Analytic/data/DataStructure/output/2-1-1-20-1.json b/tests/Analytic/data/DataStructure/output/2-1-1-20-1.json
@@ -54,13 +54,13 @@
         {
           "name": "THRD_PRTY_PRRTY_CLMS",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         },
         {
           "name": "MX_3PPC",
           "role": "Measure",
-          "type": "Number",
+          "type": "Integer",
           "nullable": true
         },
         {