5
\$\begingroup\$

I was looking through some popular questions on SO, and one of them was How do I sort a dictionary by value?. Because Python 3.6 now gives us dicts that maintain their structure, I thought I'd give this question a go. Here is my answer.

My hope is that this function eases the sorting of an iterable (tuple, list, or dict). In the latter case, you can sort either on keys or values, and it can take numeric comparison into account.

When you try using sorted on an iterable that holds e.g. strings as well as ints, it will fail. Of course you can force string comparison with str(). However, in some cases you want to do actual numeric comparison where 12 is smaller than 20 (which is not the case in string comparison). So I came up with the following. When you want explicit numeric comparison you can use the flag num_as_num which will try to do explicit numeric sorting by trying to convert all values to floats. If that succeeds, it will do numeric sorting, otherwise it'll resort to string comparison.

Looking forward to see what can be improved, efficiency wise as well as code structure wise.

def sort_iterable(iterable, sort_on=None, reverse=False, num_as_num=False):
    def _sort(i):
      try:
        if num_as_num:
          if i is None:
            _sorted = sorted(iterable, key=lambda v: float(v), reverse=reverse)
          else:
            _sorted = dict(sorted(iterable.items(), key=lambda v: float(v[i]), reverse=reverse))
        else:
          raise TypeError
      except (TypeError, ValueError):
        if i is None:
          _sorted = sorted(iterable, key=lambda v: str(v), reverse=reverse)
        else:
          _sorted = dict(sorted(iterable.items(), key=lambda v: str(v[i]), reverse=reverse))

      return _sorted

    if isinstance(iterable, list):
      sorted_list = _sort(None)
      return sorted_list
    elif isinstance(iterable, tuple):
      sorted_list = tuple(_sort(None))
      return sorted_list
    elif isinstance(iterable, dict):
      if sort_on == 'keys':
        sorted_dict = _sort(0)
        return sorted_dict
      elif sort_on == 'values':
        sorted_dict = _sort(1)
        return sorted_dict
      elif sort_on is not None:
        raise ValueError(f"Unexpected value {sort_on} for sort_on. When sorting a dict, use key or values")
    else:
      raise TypeError(f"Unexpected type {type(iterable)} for iterable. Expected a list, tuple, or dict")

Update following the answers:

  • Changed lambda expressions to just the float or str function where applicable.
  • Changed the try-except logic and how it interacts with conditionals.
  • Raise an exception when using sort_on=None when giving a dict.
  • Removed intermediary variable assignments

I am not going into backwards-or-forwards compatibility issues. If anything, it's more a fun project to improve my programming.

def sort_iterable(iterable, sort_on=None, reverse=False, num_as_num=False):
    def _sort(i):
        if num_as_num:
            try:
                if i is None:
                    return sorted(iterable, key=float, reverse=reverse)
                else:
                    return dict(sorted(iterable.items(), key=lambda v: float(v[i]), reverse=reverse))
            except TypeError:
                raise TypeError("Tried parsing as float but could not. Only use num_as_num when all items that need "
                                "to be sorted can be converted to float")
        else:
            if i is None:
                return sorted(iterable, key=str, reverse=reverse)
            else:
                return dict(sorted(iterable.items(), key=lambda v: str(v[i]), reverse=reverse))


    if isinstance(iterable, list):
        return _sort(None)
    elif isinstance(iterable, tuple):
        return tuple(_sort(None))
    elif isinstance(iterable, dict):
        if sort_on.lower() == 'keys':
            return _sort(0)
        elif sort_on.lower() == 'values':
            return _sort(1)
        else:
            raise ValueError(f"Unexpected value {sort_on} for sort_on. When sorting a dict, use keys or values")
    else:
        raise TypeError(f"Unexpected type {type(iterable)} for iterable. Expected a list, tuple, or dict")
\$\endgroup\$
5
\$\begingroup\$

From first look, the following lambda wrapping

key=lambda v: float(v)

is just pointless.

key=float

is enough. You also do not need the intermediate variables in sections:

if isinstance(iterable, list):
  return _sort(None)

You are also not showing anything if I use a dict with None as sort_on value. You could probably check inside isinstance.. dict section for it:

if sort_on not in ('keys', 'values'):  # does not take into account sort_on='KEYS' or sort_on='kEyS'
    raise hell

or

if sort_on is None or sort_on.lower() not in ('keys', 'values'):
    raise hell

You have a failsafe check for ValueError exception. I do not think this is a good behaviour. A developer should be informed that the values they provided failed the float() or str() or whatever conversion, so that they may weed out the problem instead of getting undesired behaviour.

If the above is not your intended behaviour, then ignore the rest of the following. Otherwise, you can use a local cast_to reference:

def _sort(i):
    cast_to = float if num_as_num else str
    cast_func = cast_to if i is None else lambda v: cast_to(v[i])
    cast_wrap = type(iterable)
    iter = iterable.items() if issubclass(cast_wrap, dict) else iterable
    return cast_wrap(sorted(iter, key=cast_func, reverse=reverse))

This way, you return the sorted iterable in the type it was initially provided to you, which might even be a subclass of dict/list etc.

\$\endgroup\$
  • \$\begingroup\$ Thank you for the comments. I updated my post accordingly. Your _sort function looks neat, and wrapping it into the original type is a good idea indeed. I might adopt that later, but for now it goes a bit over my head. What is the use case of issubclass here? I thought isinstance already checked for subclasses? What is the difference that is useful here? \$\endgroup\$ – Bram Vanroy Mar 3 '18 at 18:54
  • \$\begingroup\$ @BramVanroy The type(iterable) returns the class, and not the instance. Also, check this particular answer: stackoverflow.com/a/18733034/1190388 \$\endgroup\$ – hjpotter92 Mar 3 '18 at 21:13
3
\$\begingroup\$

About the Idea and design

The python way of duck typed sorting is to use sorted(). That allows

sorted([1,3,2])
tuple(sorted((1,3,2)))
sorted(['1','3','2'], key=float)
tuple(sorted(('1','3','2'), key=float))

which covers two of your 3 types as float or original type. it does not require

  • import of some handknitted function
  • guessing about capability, parameters
  • guessing about the return type
  • guessing about the depth of copy (shallow, deep, or by reference)
  • guessing about exceptions

it is clear and readable and tells all and shall not be done any other way.

Now about sorting a dict. You rely on a 3.6 implementation detail that will be guaranteed in 3.7 when released. whenever you do such a version-stunt you shall check the version and either raise an error or implement a fallback (e.g. OrderedDict). However when 3.7 will be released there might be a sort for dict implemented as well.

Your solution is most likely becoming obsolete when becoming legal.

About coding

You define an inner function for no reason introducing a lot of complexity. In your inner function you check the existence of an outer i to decide on returning a dict, that's awful.

You mix if and exception handling in if num_as_num: for control flow. your exceptclause is a completely regular case.

You shall not implement a silent fallback to sort by string (or whatever the type is) when the user requested sorting via float. this is a complete fail, hiding a user error and presenting false data as regular result.


EDIT

One important point I did miss on the first glance. You do sort optionally on float conversion or by default on str() representation. the second one is a bad idea, as you cannot sort comparable objects by their built in comparison functions. instead you force str representation which in terms of sorting is useless.

So you shall not force str but have the identity function lambda x: x. str might be a third option to sort for.

To your changed code - let's assume we have to write a function like this.

if we have a look at

def sort_iterable(iterable, sort_on=None, reverse=False, num_as_num=False):
    # [...]
    if isinstance(iterable, list):
        return _sort(None)
    elif isinstance(iterable, tuple):
        return tuple(_sort(None))
    elif isinstance(iterable, dict):
        if sort_on.lower() == 'keys':
            return _sort(0)
        elif sort_on.lower() == 'values':
            return _sort(1)
        else:
            raise ValueError(f"Unexpected value {sort_on} for sort_on. When sorting a dict, use keys or values")
    else:
        raise TypeError(f"Unexpected type {type(iterable)} for iterable. Expected a list, tuple, or dict")

we notice a parameter which has two values. we could replace that with a boolean parameter and avoid checking strings and raising an exception

def new_sort_iterable(iterable, sort_on_values=False, reverse=False, num_as_num=False):
    # [...]
    if isinstance(iterable, list):
        return _sort(None)
    elif isinstance(iterable, tuple):
        return tuple(_sort(None))
    elif isinstance(iterable, dict):
        return _sort(1) if sort_on_values else _sort(0) 
    else:
        raise TypeError(f"Unexpected type {type(iterable)} for iterable. Expected a list, tuple, or dict")

next we look at

def _sort(i):
    if num_as_num:
        try:
            if i is None:
                return sorted(iterable, key=float, reverse=reverse)
            else:
                return dict(sorted(iterable.items(), key=lambda v: float(v[i]), reverse=reverse))
        except TypeError:
            raise TypeError("Tried parsing as float but could not. Only use num_as_num when all items that need "
                            "to be sorted can be converted to float")
    else:
        if i is None:
            return sorted(iterable, key=str, reverse=reverse)
        else:
            return dict(sorted(iterable.items(), key=lambda v: str(v[i]), reverse=reverse))

here you repeat yourself, the only difference is the key. we may hide that detail, again we get rid of an explicit exception handling

convert = float if num_as_num else lambda v: v # note: default sort by v, not by str(v)
def _sort(i):
    if i is None:
        return sorted(iterable, key=convert, reverse=reverse)
    else:
        return dict(sorted(iterable.items(), key=lambda v: convert(v[i]), reverse=reverse))

BTW: your code may raise ValueError as well which goes through other than the TypeError

While we have reduced complexity there still is this ugly if i: in the inner function. I'd say we improve readability by avoiding the inner function and

def new_sort_iterable(iterable, sort_on_values=False, reverse=False, num_as_num=False):

    convert = float if num_as_num else lambda v: v

    if isinstance(iterable, list):
        return sorted(iterable, key=convert, reverse=reverse)
    elif isinstance(iterable, tuple):
        return tuple(sorted(iterable, key=convert, reverse=reverse))
    elif isinstance(iterable, dict):
        i = {False:0, True:1}[sort_on_values]
        return dict(sorted(iterable.items(), key=lambda v: convert(v[i]), reverse=reverse))
    else:
        raise TypeError(f"Unexpected type {type(iterable)} for iterable. Expected a list, tuple, or dict")

Now we could make convert a parameter instead of num_as_num

def new_sort_iterable(iterable, sort_on_values=False, reverse=False, convert=lambda v: v):

and use it like

new_sort_iterable(['1','3','2'])
new_sort_iterable(['1','3','2'], convert=float)
new_sort_iterable(['1','3','2'], convert=str)

Still there are docstrings missing ...

\$\endgroup\$
  • \$\begingroup\$ I agree about the first part, even though you always get the same type back that you put in. The code is specifically written for 3.6 or later, so I won't implement any fallbacks. That's outside the scope of this li'l project. What is wrong with mixing conditional statements and exception handling? I like that it results in DRY code, even though it's not really semantically correct. Also, what is wrong with an inner function? The i is not outer, and passed internally so it's always in control. Don't see what's so awful about that. \$\endgroup\$ – Bram Vanroy Mar 3 '18 at 18:19
  • \$\begingroup\$ DRY btw would be a good reason for an inner function. however the code you want to reuse shall not be used in case of an exception. this hides a user error and gives wrong results. try to sort [' 1','-3','- 2'] on float and you understand. it was never meant to be used on python < 3.6 reminds me on Mars Climate Orbiter. again this fails completely silent and delivers an unordered dict. both these silent fails have absolutely deadly potential. \$\endgroup\$ – stefan Mar 3 '18 at 20:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.