Repository: census-instrumentation/opencensus-python
Pull request #180 (2018-05-22 07:55:12)

Use enum-based aggregation types in stats
I think it would be nice to use enum types as aggregation types. Also, it is not required to include separate aggregation_type property function for each aggregation since it inherits from the parent 'BaseAggregation' class. 


Patch:
diff --git a/opencensus/stats/aggregation.py b/opencensus/stats/aggregation.py
index ff7f09e..9bced56 100644
--- a/opencensus/stats/aggregation.py
+++ b/opencensus/stats/aggregation.py
@@ -16,6 +16,21 @@ from opencensus.stats import bucket_boundaries
 from opencensus.stats import aggregation_data
 
 
+class Type(object):
+    """ The type of aggreation function used on a View.
+
+    Attributes:
+      NONE (int): The aggreation type of the view is 'unknown'.
+      SUM (int): The aggreation type of the view is 'sum'.
+      COUNT (int): The aggreation type of the view is 'count'.
+      DISTRIBUTION (int): The aggreation type of the view is 'distribution'.
+    """
+    NONE = 0
+    SUM = 1
+    COUNT = 2
+    DISTRIBUTION = 3
+
+
 class BaseAggregation(object):
     """Aggregation describes how the data collected is aggregated by type of
     aggregation and buckets
@@ -25,9 +40,12 @@ class BaseAggregation(object):
     :param buckets: list of endpoints if the aggregation represents a
                     distribution
 
+    :type aggregation_type: :class:`~opencensus.stats.aggregation.Type`
+    :param aggregation_type: represents the type of this aggregation
+
     """
-    def __init__(self, buckets=None):
-        self._aggregation_type = "none"
+    def __init__(self, buckets=None, aggregation_type=Type.NONE):
+        self._aggregation_type = aggregation_type
         self._buckets = buckets or []
 
     @property
@@ -48,19 +66,15 @@ class SumAggregation(BaseAggregation):
     :type sum: int or float
     :param sum: the sum of the data collected and aggregated
 
-    """
-    def __init__(self, sum=None):
-        super(SumAggregation, self).__init__()
-        self._aggregation_type = "sum"
-        if sum is not None:
-            self._sum = aggregation_data.SumAggregationDataFloat(sum_data=sum)
-        else:
-            self._sum = aggregation_data.SumAggregationDataFloat(sum_data=0)
 
-    @property
-    def aggregation_type(self):
-        """The aggregation type of the current aggregation"""
-        return self._aggregation_type
+    :type aggregation_type: :class:`~opencensus.stats.aggregation.Type`
+    :param aggregation_type: represents the type of this aggregation
+
+    """
+    def __init__(self, sum=None, aggregation_type=Type.SUM):
+        super(SumAggregation, self).__init__(aggregation_type=aggregation_type)
+        self._sum = aggregation_data.SumAggregationDataFloat(
+            sum_data=float(sum or 0))
 
     @property
     def sum(self):
@@ -75,17 +89,15 @@ class CountAggregation(BaseAggregation):
     :type count: int
     :param count: represents the count of this aggregation
 
+    :type aggregation_type: :class:`~opencensus.stats.aggregation.Type`
+    :param aggregation_type: represents the type of this aggregation
+
     """
-    def __init__(self, count=0):
-        super(CountAggregation, self).__init__()
-        self._aggregation_type = "count"
+    def __init__(self, count=0, aggregation_type=Type.COUNT):
+        super(CountAggregation, self).__init__(
+            aggregation_type=aggregation_type)
         self._count = aggregation_data.CountAggregationData(count)
 
-    @property
-    def aggregation_type(self):
-        """The aggregation type of the current aggregation"""
-        return self._aggregation_type
-
     @property
     def count(self):
         """The count of the current aggregation"""
@@ -103,17 +115,19 @@ class DistributionAggregation(BaseAggregation):
     :type distribution: histogram
     :param distribution: histogram of the values of the population
 
+    :type aggregation_type: :class:`~opencensus.stats.aggregation.Type`
+    :param aggregation_type: represents the type of this aggregation
+
     """
-    def __init__(self, boundaries=None, distribution=None):
-        super(DistributionAggregation, self).__init__(boundaries)
-        self._aggregation_type = "distribution"
+    def __init__(
+            self,
+            boundaries=None,
+            distribution=None,
+            aggregation_type=Type.DISTRIBUTION):
+        super(DistributionAggregation, self).__init__(
+            buckets=boundaries, aggregation_type=aggregation_type)
         self._boundaries = bucket_boundaries.BucketBoundaries(boundaries)
-        self._distribution = distribution if distribution is not None else {}
-
-    @property
-    def aggregation_type(self):
-        """The aggregation type of the current aggregation"""
-        return self._aggregation_type
+        self._distribution = distribution or {}
 
     @property
     def boundaries(self):


Test patch:
diff --git a/tests/unit/stats/test_aggregation.py b/tests/unit/stats/test_aggregation.py
index 7a6db45..9fc1a14 100644
--- a/tests/unit/stats/test_aggregation.py
+++ b/tests/unit/stats/test_aggregation.py
@@ -22,7 +22,7 @@ class TestBaseAggregation(unittest.TestCase):
     def test_constructor_defaults(self):
         base_aggregation = aggregation_module.BaseAggregation()
 
-        self.assertEqual("none", base_aggregation.aggregation_type)
+        self.assertEqual(aggregation_module.Type.NONE, base_aggregation.aggregation_type)
         self.assertEqual([], base_aggregation.buckets)
 
     def test_constructor_explicit(self):
@@ -30,7 +30,7 @@ class TestBaseAggregation(unittest.TestCase):
         buckets = ["test"]
         base_aggregation = aggregation_module.BaseAggregation(buckets=buckets)
 
-        self.assertEqual("none", base_aggregation.aggregation_type)
+        self.assertEqual(aggregation_module.Type.NONE, base_aggregation.aggregation_type)
         self.assertEqual(["test"], base_aggregation.buckets)
 
 
@@ -40,7 +40,7 @@ class TestSumAggregation(unittest.TestCase):
         sum_aggregation = aggregation_module.SumAggregation()
 
         self.assertEqual(0, sum_aggregation.sum.sum_data)
-        self.assertEqual("sum", sum_aggregation.aggregation_type)
+        self.assertEqual(aggregation_module.Type.SUM, sum_aggregation.aggregation_type)
 
     def test_constructor_explicit(self):
         sum = 1
@@ -48,7 +48,7 @@ class TestSumAggregation(unittest.TestCase):
         sum_aggregation = aggregation_module.SumAggregation(sum=sum)
 
         self.assertEqual(1, sum_aggregation.sum.sum_data)
-        self.assertEqual("sum", sum_aggregation.aggregation_type)
+        self.assertEqual(aggregation_module.Type.SUM, sum_aggregation.aggregation_type)
 
 
 class TestCountAggregation(unittest.TestCase):
@@ -57,7 +57,7 @@ class TestCountAggregation(unittest.TestCase):
         count_aggregation = aggregation_module.CountAggregation()
 
         self.assertEqual(0, count_aggregation.count.count_data)
-        self.assertEqual("count", count_aggregation.aggregation_type)
+        self.assertEqual(aggregation_module.Type.COUNT, count_aggregation.aggregation_type)
 
     def test_constructor_explicit(self):
         count = 4
@@ -65,7 +65,7 @@ class TestCountAggregation(unittest.TestCase):
         count_aggregation = aggregation_module.CountAggregation(count=count)
 
         self.assertEqual(4, count_aggregation.count.count_data)
-        self.assertEqual("count", count_aggregation.aggregation_type)
+        self.assertEqual(aggregation_module.Type.COUNT, count_aggregation.aggregation_type)
 
 
 class TestDistributionAggregation(unittest.TestCase):
@@ -75,7 +75,7 @@ class TestDistributionAggregation(unittest.TestCase):
 
         self.assertEqual([], distribution_aggregation.boundaries.boundaries)
         self.assertEqual({}, distribution_aggregation.distribution)
-        self.assertEqual("distribution", distribution_aggregation.aggregation_type)
+        self.assertEqual(aggregation_module.Type.DISTRIBUTION, distribution_aggregation.aggregation_type)
 
     def test_constructor_explicit(self):
         boundaries = ["test"]
@@ -84,4 +84,4 @@ class TestDistributionAggregation(unittest.TestCase):
 
         self.assertEqual(["test"], distribution_aggregation.boundaries.boundaries)
         self.assertEqual({1: "test"}, distribution_aggregation.distribution)
-        self.assertEqual("distribution", distribution_aggregation.aggregation_type)
+        self.assertEqual(aggregation_module.Type.DISTRIBUTION, distribution_aggregation.aggregation_type)