0

I am trying to make some error-catching code. It will always execute the else block of the first if statement, no matter what the function's input was. Why is it doing this?

This is the error-catching code:

def rgbtohex(r=0, g=0, b=0):
    '''Converts RGB values to hecadeximal values. Supports 3 integers, one list, or one tuple.'''
    if type(r) == 'int' and type(g) == 'int' and type(b) == 'int':
        if r > 255 or g > 255 or b > 255:
            raise ValueError('one or more arguments are above 255.')
    elif type(r) == 'list' or type(r) == 'tuple':
        if r[0] > 255 or r[1] > 255 or r[2] > 255:
            raise ValueError('one or more index values are above 255.')
        if g == 0 and b == 0:
            r = r[0]
            g = r[1]
            b = r[2]
        else:
            raise TypeError('rgb values not integers, single list, or single tuple.')
        return
    else:
        raise TypeError('one or more arguments are not integers.')
    ...

4 Answers4

3

In Python, the integer type is int, not the string "int".

Remove the quotes.

Ditto for tuple and list.

It's an easy mistake to make, because other languages like JavaScript and Lua use strings to indicate types. But in Python (as in Ruby), types are actual objects, and referred to by identifiers.

Soapbox

Something to consider: I see that you are trying to make a function that users can pass either three integers, or a tuple, or a list. You are trying to allow your callers some flexibility here, which is commendable, but what you've ended up with is something that

  1. does typechecking of arguments, which isn't super Pythonic, and
  2. uses a parameter called r for the list or tuple!

The second part means that someone can call

rgbtohex(r=[21,128,123])

which is kind of weird.

What I would do is define your function solely as

def rgbtohex(r = 0, g = 0, b = 0):
    ...

And if your user has a list or tuple, they will know to unpack and call like this:

my_color = [21,128,123]
rgbtohex(*myColor)

Here's how I'd do this:

def rgbtohex(r=0, g=0, b=0):
    if not all(c in range(256) for c in (r, g, b)):
        raise ValueError('Components must be in range 0...255')
    return '#{:02x}{:02x}{:02x}'.format(r, g, b)

assert(rgbtohex() == '#000000')
assert(rgbtohex(9, 20, 255) == '#0914ff')
assert(rgbtohex(9, b=20, g=192) == '#09c014')
assert(rgbtohex(*[11, 0, 0]) == '#0b0000')
// Negative tests left as an exercise for the reader ;-)
Ray Toal
  • 86,166
  • 18
  • 182
  • 232
  • What would you suggest I rename the parameter `r` to? – Thomas Laskowski Apr 09 '17 at 05:23
  • There is no good name for this parameter, because you are using it both as the first of three integers AND as the sole tuple or array. This dual use of a parameter is a real red flag, or "code smell" and really TBH should be avoided. I think it's great you want to give some flexibility to those who are calling your function, but Python is a modern language that _already provides cool, flexible features_. IMHO you should take the suggestion in my answer and completely avoid type checking. Support only the 3-parameter version with r, g, b. Your users will use the `*` if they have a list. – Ray Toal Apr 09 '17 at 08:03
2

even though If is true?

Never assume this. The code doesn't lie.

type(r) == 'int' will never be true when type(r) is actually an int (no quotes)

Try it print(type(r) == 'int')


Don't string your types.

For example, though, isinstance(r, int) does look better

As far as checking for lists, sets, tuples, etc.

In Python, how do I determine if an object is iterable?

Community
  • 1
  • 1
OneCricketeer
  • 179,855
  • 19
  • 132
  • 245
  • Thank you so much for your answer. My confusion arose from executing `print(type(1))` and the return being ``. I then made the assumption that the integer's typing would be in `string` format, because I wasn't aware that `type` was an actual type (if that makes any sense). Simple beginners mistakes on my part. – Thomas Laskowski Apr 09 '17 at 04:22
0

You could use isinstance() method because comparing an int to a str will be always be False:

So you can change your condition

if type(r) == 'int' and type(g) == 'int' and type(b) == 'int':
    # process

to :

if isinstance(r, int) and isinstance(g, int) and isinstance(b, int):
    # process

Do, the same for the other conditions.

Chiheb Nexus
  • 9,104
  • 4
  • 30
  • 43
0

The type of the value returned by type() is of type 'type.' To check if x is an integer, use this code:

if type(x) == type(5):
    print("X is an integer.")
Feather Feet
  • 131
  • 1
  • 9