7
\$\begingroup\$

I made this utility function to check for parameters. All it does is checking whether a given variable is an int, or a float/str that represent exactly an int (ie. no decimal part or NaN), and casting to that int (raising a ValueError exception on failure).

I'm not very experienced with Python (especially Python 3+), so I'm still learning and trying to improve my programming skills.

I'm open to any critics, improvements, and bad/good practices tips. Also I'm trying to learn how to properly document Python code (my IDE is PyCharm and hints me to use reStructuredText docblocks, and default Python3 typing hints).

I'm using Python3.6 for the following code:

util.py

from typing import Union


def int_exact_cast(n: Union[int, float, str]) -> int:
    """
    Tries to cast parameter to int without losing decimal part or raise an exception
    :param Union[int, float] n: The value to be converted
    :type n: Union[int, float]
    :return: The converted int value
    :rtype: int
    :raises ValueError: raises ValueError exception if the parameter can't be casted exactly to int
    """

    t = type(n)

    if t is int:
        return n  # return immediately if is already int (optimization)

    if not (t is str or t is float):
        raise ValueError('Parameter must be an int, str or float')

    f = float(n)

    if not f.is_integer():  # not checking for math.nan because in that case is_integer() returns False
        raise ValueError('Parameter contains a decimal part')

    return int(f)
\$\endgroup\$
  • 1
    \$\begingroup\$ You could check for instances, but not for exact types. Also if param type is not expected type then you should raise TypeError not ValueError. Check PEP257 for a docstring convention \$\endgroup\$ – Alex May 19 '17 at 12:20
  • \$\begingroup\$ @Alex thanks Alex. How would you change it? Do you mean I should use isinstance() instead of type()? \$\endgroup\$ – Zorgatone May 19 '17 at 12:54
  • 1
    \$\begingroup\$ A discussion about isinstance() vs. type() is here: stackoverflow.com/questions/1549801/… but in summary isinstance() supports inheritence, so if you were using classes of Cat and it inherited from class Animal, then and instance of cat in the var tabby means that tabby.isinstance(Cat(), Animal) is True. For what you are doing here, type will work. \$\endgroup\$ – srattigan Sep 1 '17 at 0:05
  • \$\begingroup\$ Ok thanks @srattigan. Would it still be a good practice for this example? Here I'm checking for "primitive" types so there is no inheritance going on \$\endgroup\$ – Zorgatone Sep 1 '17 at 8:54
  • \$\begingroup\$ Python doesn't have primitive types like Java does. If you use the type(2) in Python 2.x it will return <type 'int'>, but doing the same in Python3.x it will return <class 'int'>, so a type is really asking for the class of the object, and all Python classes inherit from a base object class => there is some inheritence going on "under the hood" so to speak. You are only checking built-in types- or should I say built-in classes- as distinct from user-defined classes, so I would use it as you have here. I could not find anything definitive to say otherwise. \$\endgroup\$ – srattigan Sep 1 '17 at 9:08
2
\$\begingroup\$

I know this question/post is old, but...


Use isinstance rather than directly storing and then checking the type() output.

We can get rid of t and two of your if statements to check type by using isinstance instead, and check against all three acceptable types:

if not (isinstance(n, (float, str, int))):
    raise ValueError('Parameter must be an int, str or float')

Just a thought on this, though: If something provided is not an int, str, or float, then it's an invalid type, and should raise a TypeError, rather than a ValueError, because TypeError is more generally used when you're passing an invalid object type into a function that can't handle it. Therefore, your raise ValueError("...") for cases where the parameter is not an int, str, or float should probably be a TypeError with the same error message.

As was mentioned, isinstance will not behave right if you are using inherited classes which are subclasses of other primary classes. However, consider that subclasses building upon other immutable classes are rare and as long as you aren't testing for instances of a bare object, this should suit your purposes.


The final code is this:

from typing import Union


def int_exact_cast(n: Union[int, float, str]) -> int:
    """
    Tries to cast parameter to int without losing decimal part or raise an exception
    :param Union[int, float] n: The value to be converted
    :type n: Union[int, float]
    :return: The converted int value
    :rtype: int
    :raises ValueError: raises ValueError exception if the parameter can't be casted exactly to int
    """

    if not (isinstance(n, (float, str, int))):
        raise ValueError('Parameter must be an int, str or float')

    f = float(n)

    if not f.is_integer():
        raise ValueError('Parameter contains a decimal part')

    return int(f)

Just keep in mind that if the string can't be converted to a number (such as if the string is 'I Am A Teapot'), then you get a different ValueError from the conversion system, because it's Not a Number. (Which is why you don't need the NaN check with this method, but you have to be careful with what you're working with)

\$\endgroup\$

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.