1
\$\begingroup\$

I have not tried decorators yet or functions within functions, but this current un-pythonic method seems a little convoluted. I don't like how I need to repeat myself when I check the return_type 3 times.

Any suggestions are welcome, especially if the repetion can be dealt with in an elegant way. Please note that I am not that interested in the numerous reference to test if an object is a number as I believe this part is incorporated within my solution. I am interested in addressing the 3x duplication to deal with the return type. Moreover, while I appreciate the locale method is the more rigorous way of dealing with internationalisation, I prefer the simplicity of allowing the caller more flexibility in choosing the characters.

Also, some people on Stack Overflow just posted trivial solutions to the smaller boolean is_number function that has been asked many times.

def is_number(obj, thousand_sep=',', decimal_sep=None, return_type='b'):
    """ determines if obj is numeric.

    if return_type = b, returns a boolean True/False
    otherwise, it returns the numeric value

    Examples
    --------
    >>> is_number(3)
    True
    >>> is_number('-4.1728')
    True
    >>> is_number('-4.1728', return_type='n')
    -4.1728
    >>> is_number(-5.43)
    True
    >>> is_number("20,000.43")
    True
    >>> is_number("20.000,43", decimal_sep=",", thousand_sep=",")
    True
    >>> is_number("20.000,43", decimal_sep=",", thousand_sep=".", return_type="n")
    20000.43
    >>> is_number('Four')
    False
    >>> is_number('Four', return_type='n')
    """
    try:
        if is_string(obj):
            if decimal_sep is None:
                value = float(obj.replace(thousand_sep, ""))
            else:
                value = float(obj.replace(thousand_sep, "").replace(decimal_sep, "."))
            if return_type.lower() == 'b':
                return True
            else:
                return value
        else:
            value = float(obj)
            if return_type.lower() == 'b':
                return True
            else:
                return value
    except ValueError:
        return False
        if return_type.lower() == 'b':
            return False
        else:
            return None
\$\endgroup\$
3
\$\begingroup\$

Default arguments and returning

I have a few issues with your default arguments. Firstly, you're expecting the function to work with commas and periods so use those as the defaults! Also, the default behaviour is to return a bool so don't use a special letter to indicate this, use a boolean to explicitly choose non-default behaviour. Your definition should be something like:

def is_number(obj, thousand_sep=',', decimal_sep='.', return_the_value=False):

You should use return_the_value to choose what to return on the return line itself. Assuming that value is either valid or None, your return statements could look like this:

return value if return_the_value else not value is None

This removes the need for your many if else return structures.

Dealing with non-strings

Your function can handle two cases: either the obj is string-like and might have separators or it can cast into a float immediately. Otherwise it's invalid. You only need to do any real work in the string case. The python idiom for determining string-ness is isinstance(obj, basestring), use this instead of your own is_string function.

def is_number(...):
   if not isinstance(obj, basestring):
       try:
           value = float(obj)
       except ValueError:
           value = None
       return value if return_the_value else not value is None
   else:
      # Deal with the string

Dealing with strings

Assume the string is a valid number. What is the minimum manipulation you can do to it to turn it into a valid input for float? If you come up with a scheme that works for any valid number, then if float still fails - you can be happy that the number wasn't valid to start with!

You're sort of thinking along these lines already but you're removing the thousands separator blindly, leaving your function vulnerable to inputs like ',,,,,,,,,,,,0,,,,,,,,,,,,,,'.

Valid numbers use decimal and thousands separators in very specific ways and you need to test for this behaviour. Here's some rules:

  • There can only be zero or one decimal character
  • Thousands separators can only be in the integer part of a decimal number
  • From the decimal, working left, the thousands separator appears after every block of 3 digits

The decimal is easiest to deal with. A valid number string can be split into an integer part and a decimal part. The decimal part should be pure numbers - no separators. The string split method can be used to split the number into these two parts, just be sure to handle the empty string decimal_sep

parts = obj.split(decimal_sep) if decimal_sep else [obj]

A valid number will require no further manipulation of the decimal part so we just need to focus on the integer, and its thousands separators. All you need to check is that, if separators are being used, they are in all the right places.

integer_part = parts[0]
if thousand_sep and thousand_sep in integer_part:
    # The number is using thousand separators
    chunks = integer_part.split(thousand_sep)
    if len(chunks[0].lstrip('-')) > 3 or any(len(chunk) != 3 for chunk in chunks[1:]):
        return None if return_a_value else False
    parts[0] = ''.join(chunks)  # Rejoin the number without separators

Finally, rejoin the integer and decimal parts with a period separator and try to float them:

try:
    value = float('.'.join([integer_part] + parts[1:])
except ValueError:
    # not a number!

Final result

def is_number(obj, thousand_sep=',', decimal_sep='.', return_the_value=False):
    if isinstance(obj, basestring):
        parts = obj.split(decimal_sep) if decimal_sep else [obj]
        integer_part = parts[0]
        if thousand_sep and thousand_sep in integer_part:
            chunks = integer_part.split(thousand_sep)
            if len(chunks[0].lstrip('-')) > 3 or any(len(c) != 3 for c in chunks[1:]):
                return None if return_the_value else False
            parts[0] = ''.join(chunks)
        obj = '.'.join(parts)
    try:
        value = float(obj)
    except ValueError:
        value = None
    return value if return_the_value else value is not None
\$\endgroup\$
  • \$\begingroup\$ Just as the original was too free with where the thousand_sep could occur, your code may be too strict. Notably, China appears to use groups of 4, and India uses one group of 3 but then groups of 2, so the chunk length tests would reject their conventions. Summarized here. It's unclear how flexible the original intent really was. \$\endgroup\$ – Michael Urman Dec 7 '13 at 21:46
  • \$\begingroup\$ That's a good point but it's not a thousand_sep if it doesn't break into threes. To read the Chinese style you'd need a tenthou_sep for instance. It wouldn't be hard to allow for Chinese and Indian styles through another argument. I feel that inputs like 123,4,32.1 shouldn't count though. \$\endgroup\$ – ejrb Dec 7 '13 at 22:00
2
\$\begingroup\$

I am disregarding some questionable code (such as the fact that '3,,,4' would be handled identically to '34', and so forth), and as you mention, the fact that locale's atof is probably more fitting than write-your-own. I am doing so because, worse than those, I really don't like the return_type parameter and its usage.

I would strongly suggest separating this into two functions. For example a to_number which acts like return_type='n', or even throws an exception instead of returning None, and an is_number which calls to_number and converts the exception or None to False, and other values to True. Each function would have very simple usage thanks to very simple definitions, and their code would become simpler accordingly.

\$\endgroup\$
  • \$\begingroup\$ and is_number would just be def is_number(...): return to_number(...) is not None. \$\endgroup\$ – RemcoGerlich Dec 7 '13 at 19:23

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.