Merge "Handle null values when sorting"
This commit is contained in:
commit
a5bdcc6e78
|
@ -13,7 +13,7 @@
|
|||
"""Application base class for providing a list of data as output.
|
||||
"""
|
||||
import abc
|
||||
import operator
|
||||
import logging
|
||||
|
||||
from . import display
|
||||
|
||||
|
@ -22,6 +22,8 @@ class Lister(display.DisplayCommandBase, metaclass=abc.ABCMeta):
|
|||
"""Command base class for providing a list of data as output.
|
||||
"""
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
@property
|
||||
def formatter_namespace(self):
|
||||
return 'cliff.formatter.list'
|
||||
|
@ -63,8 +65,25 @@ class Lister(display.DisplayCommandBase, metaclass=abc.ABCMeta):
|
|||
if parsed_args.sort_columns and self.need_sort_by_cliff:
|
||||
indexes = [column_names.index(c) for c in parsed_args.sort_columns
|
||||
if c in column_names]
|
||||
if indexes:
|
||||
data = sorted(data, key=operator.itemgetter(*indexes))
|
||||
for index in indexes[::-1]:
|
||||
try:
|
||||
# We need to handle unset values (i.e. None) so we sort on
|
||||
# multiple conditions: the first comparing the results of
|
||||
# an 'is None' type check and the second comparing the
|
||||
# actual value. The second condition will only be checked
|
||||
# if the first returns True, which only happens if the
|
||||
# returns from the 'is None' check on the two values are
|
||||
# the same, i.e. both None or both not-None
|
||||
data = sorted(
|
||||
data, key=lambda k: (k[index] is None, k[index]),
|
||||
)
|
||||
except TypeError:
|
||||
# Simply log and then ignore this; sorting is best effort
|
||||
self.log.warning(
|
||||
"Could not sort on field '%s'; unsortable types",
|
||||
parsed_args.sort_columns[index],
|
||||
)
|
||||
|
||||
(columns_to_include, selector) = self._generate_columns_and_selector(
|
||||
parsed_args, column_names)
|
||||
if selector:
|
||||
|
|
|
@ -32,16 +32,15 @@ class FauxFormatter(object):
|
|||
|
||||
class ExerciseLister(lister.Lister):
|
||||
|
||||
data = [('a', 'A'), ('b', 'B'), ('c', 'A')]
|
||||
|
||||
def _load_formatter_plugins(self):
|
||||
return {
|
||||
'test': FauxFormatter(),
|
||||
}
|
||||
|
||||
def take_action(self, parsed_args):
|
||||
return (
|
||||
parsed_args.columns,
|
||||
[('a', 'A'), ('b', 'B'), ('c', 'A')],
|
||||
)
|
||||
return (parsed_args.columns, self.data)
|
||||
|
||||
|
||||
class ExerciseListerCustomSort(ExerciseLister):
|
||||
|
@ -49,6 +48,16 @@ class ExerciseListerCustomSort(ExerciseLister):
|
|||
need_sort_by_cliff = False
|
||||
|
||||
|
||||
class ExerciseListerNullValues(ExerciseLister):
|
||||
|
||||
data = ExerciseLister.data + [(None, None)]
|
||||
|
||||
|
||||
class ExerciseListerDifferentTypes(ExerciseLister):
|
||||
|
||||
data = ExerciseLister.data + [(1, 0)]
|
||||
|
||||
|
||||
class TestLister(base.TestBase):
|
||||
|
||||
def test_formatter_args(self):
|
||||
|
@ -111,6 +120,43 @@ class TestLister(base.TestBase):
|
|||
data = list(args[1])
|
||||
self.assertEqual([['a', 'A'], ['b', 'B'], ['c', 'A']], data)
|
||||
|
||||
def test_sort_by_column_with_null(self):
|
||||
test_lister = ExerciseListerNullValues(mock.Mock(), [])
|
||||
parsed_args = mock.Mock()
|
||||
parsed_args.columns = ('Col1', 'Col2')
|
||||
parsed_args.formatter = 'test'
|
||||
parsed_args.sort_columns = ['Col2', 'Col1']
|
||||
|
||||
test_lister.run(parsed_args)
|
||||
|
||||
f = test_lister._formatter_plugins['test']
|
||||
args = f.args[0]
|
||||
data = list(args[1])
|
||||
self.assertEqual(
|
||||
[['a', 'A'], ['c', 'A'], ['b', 'B'], [None, None]], data)
|
||||
|
||||
def test_sort_by_column_with_different_types(self):
|
||||
test_lister = ExerciseListerDifferentTypes(mock.Mock(), [])
|
||||
parsed_args = mock.Mock()
|
||||
parsed_args.columns = ('Col1', 'Col2')
|
||||
parsed_args.formatter = 'test'
|
||||
parsed_args.sort_columns = ['Col2', 'Col1']
|
||||
|
||||
with mock.patch.object(lister.Lister, 'log') as mock_log:
|
||||
test_lister.run(parsed_args)
|
||||
|
||||
f = test_lister._formatter_plugins['test']
|
||||
args = f.args[0]
|
||||
data = list(args[1])
|
||||
# The output should be unchanged
|
||||
self.assertEqual(
|
||||
[['a', 'A'], ['b', 'B'], ['c', 'A'], [1, 0]], data)
|
||||
# but we should have logged a warning
|
||||
mock_log.warning.assert_has_calls([
|
||||
mock.call("Could not sort on field '%s'; unsortable types", col)
|
||||
for col in parsed_args.sort_columns
|
||||
])
|
||||
|
||||
def test_sort_by_non_displayed_column(self):
|
||||
test_lister = ExerciseLister(mock.Mock(), [])
|
||||
parsed_args = mock.Mock()
|
||||
|
|
|
@ -0,0 +1,8 @@
|
|||
---
|
||||
fixes:
|
||||
- |
|
||||
Sorting output using the ``--sort-column`` option will now handle ``None``
|
||||
values. This was supported implicitly in Python 2 but was broken in the
|
||||
move to Python 3. In addition, requests to sort a column containing
|
||||
non-comparable types will now be ignored. Previously, these request would
|
||||
result in a ``TypeError``.
|
Loading…
Reference in New Issue