Skip to content

Commit

Permalink
Merge pull request wireservice#706 from Mr-F/master
Browse files Browse the repository at this point in the history
Fix divide by zero when calculating Means
  • Loading branch information
jpmckinney authored Mar 31, 2020
2 parents 8165245 + 02682a0 commit b2d1ebb
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
7 changes: 5 additions & 2 deletions agate/aggregations/mean.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ def validate(self, table):

def run(self, table):
column = table.columns[self._column_name]
num_of_values = len(column.values_without_nulls())
# If there are no non-null columns then return null.
if num_of_values == 0:
return None

sum_total = self._sum.run(table)

return sum_total / len(column.values_without_nulls())
return sum_total / num_of_values
23 changes: 17 additions & 6 deletions tests/test_aggregations.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,17 +223,17 @@ def test_sum(self):
class TestNumberAggregation(unittest.TestCase):
def setUp(self):
self.rows = (
(Decimal('1.1'), Decimal('2.19'), 'a'),
(Decimal('2.7'), Decimal('3.42'), 'b'),
(None, Decimal('4.1'), 'c'),
(Decimal('2.7'), Decimal('3.42'), 'c')
(Decimal('1.1'), Decimal('2.19'), 'a', None),
(Decimal('2.7'), Decimal('3.42'), 'b', None),
(None, Decimal('4.1'), 'c', None),
(Decimal('2.7'), Decimal('3.42'), 'c', None)
)

self.number_type = Number()
self.text_type = Text()

self.column_names = ['one', 'two', 'three']
self.column_types = [self.number_type, self.number_type, self.text_type]
self.column_names = ['one', 'two', 'three', 'four']
self.column_types = [self.number_type, self.number_type, self.text_type, self.number_type]

self.table = Table(self.rows, self.column_names, self.column_types)

Expand Down Expand Up @@ -284,6 +284,17 @@ def test_mean(self):

self.assertEqual(Mean('two').run(self.table), Decimal('3.2825'))

def test_mean_all_nulls(self):
"""
Test to confirm mean of only nulls doesn't cause a critical error.
The assumption here is that if you attempt to perform a mean
calculation, on a column which contains only null values, then a null
value should be returned to the caller.
:return:
"""
self.assertIsNone(Mean('four').run(self.table))

def test_mean_with_nulls(self):
warnings.simplefilter('ignore')

Expand Down

0 comments on commit b2d1ebb

Please sign in to comment.