Skip to content

Commit

Permalink
Merge with pull request wireservice#706 by @Mr-F https://github.com/Mr-F
Browse files Browse the repository at this point in the history
  • Loading branch information
lcorbasson committed Dec 5, 2019
1 parent f1caa8d commit e888c19
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 10 deletions.
2 changes: 1 addition & 1 deletion agate/aggregations/max.py
Expand Up @@ -32,7 +32,7 @@ def validate(self, table):
if not (isinstance(column.data_type, Number) or
isinstance(column.data_type, Date) or
isinstance(column.data_type, DateTime)):
raise DataTypeError('Max can only be applied to columns containing DateTime orNumber data.')
raise DataTypeError('Max can only be applied to columns containing DateTime or Number data.')

def run(self, table):
column = table.columns[self._column_name]
Expand Down
7 changes: 5 additions & 2 deletions agate/aggregations/mean.py
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
2 changes: 1 addition & 1 deletion agate/aggregations/min.py
Expand Up @@ -32,7 +32,7 @@ def validate(self, table):
if not (isinstance(column.data_type, Number) or
isinstance(column.data_type, Date) or
isinstance(column.data_type, DateTime)):
raise DataTypeError('Min can only be applied to columns containing DateTime orNumber data.')
raise DataTypeError('Min can only be applied to columns containing DateTime or Number data.')

def run(self, table):
column = table.columns[self._column_name]
Expand Down
23 changes: 17 additions & 6 deletions tests/test_aggregations.py
Expand Up @@ -211,17 +211,17 @@ def test_max(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 @@ -272,6 +272,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 e888c19

Please sign in to comment.