Check parameters of a function in a Python

Question

For example, I have a function, that generates procedural noise

def procedural_noise(width, height, seed):
   ...

All of the parameters of this function should be positive. I suppose, that me need to check it and throw exception if on of parameters is non-positive. Is it a good (pythonic way) approach?

Let us suppose, that I am right. Which is the best way to check parameters?

I can write the checkers for each of parameters:

def procedural_noise(width, height, seed):
    if width <= 0:
        raise ValueError("Width should be positive")
    if height <= 0:
        raise ValueError("Height should be positive")
    if seed <= 0:
        raise ValueError("Seed should be positive")
    ...

It should be clearly for programmer when he will get exception, what he should to correct, but it's not good-looking in my opinion.

The following code is easier, but it is too far from ideal:

def procedural_noise(width, height, seed):
    if width <= 0 or height <= 0 or seed <= 0:
        raise ValueError("All of the parameters should be positive")
    ...

The last question: which is the best way to write tests with unittest framework, that checks types of parameters and their values?

I can write the following function in a test class:

def test_positive(self):
    self.assertRaises(ValueError, main.procedural_noise, -10, -10, 187)

Is it a correct solution?

UPD: I upvoted all answers, because each of them have a useful information for me, but I can't select the best answers of them (I suppose that it is fair to select most voted question tomorrow)

Something is wrong here - zero is not positive, but no error would be raised :) — Messa
– Messa, Commented Apr 11, 2015 at 22:29

Messa · Accepted Answer · 2015-04-11 23:12:28Z

6

Also this could be a nice use case for function annotations (in Python 3). Example:

import inspect
from functools import wraps    

def positive(name, value):
    if value < 0:
        raise ValueError(name + " must be positive")

def check_params(f):
    signature = inspect.signature(f)
    @wraps(f)
    def wrapper(*args, **kwargs):
        bound_arguments = signature.bind(*args, **kwargs)
        for name, value in bound_arguments.arguments.items():
            annotation = signature.parameters[name].annotation
            if annotation is inspect.Signature.empty:
                continue
            annotation(name, value)
        return f(*args, **kwargs)
    return wrapper

@check_params
def procedural_noise(width: positive, height: positive, seed: positive):
    pass # ...

It's a little bit of inspect-fu in the check_params decorator (inspired by github.com/ceronman/typeannotations), but provides pretty nice and flexible way to check function arguments - without any ifs, fors or other noise-code in the type-checked functions.

edited Apr 11, 2015 at 23:12

answered Apr 11, 2015 at 22:46

Messa

25.4k10 gold badges77 silver badges101 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Queue Overflow Over a year ago

Thanks a lot, it is very interesting solution. In particular, because I haven't encountered such nice application of function annotations before.

Messa · Accepted Answer · 2015-04-11 23:06:37Z

6

I would do it this way:

def procedural_noise(width, height, seed):
    check_positive(width, "width")
    check_positive(height, "height")
    check_positive(seed, "seed")

def check_positive(value, name):
    if value < 0:
        raise ValueError(name + " must be positive")

Another idea - a little "hack":

def procedural_noise(width, height, seed):
    check_positive(width=width)
    check_positive(height=height)
    check_positive(seed=seed)

def check_positive(**kwargs):
    for name, value in kwargs.items():
        if value < 0:
            raise ValueError(name + " must be positive")

Which, well, could be called also this way:

def procedural_noise(width, height, seed):
    check_positive(width=width, height=height, seed=seed)

This is almost the same as in other answers, but this way the original function procedural_noise is kept pretty clean of any argument processing except the very basic information. It's more semantic :)

edited Apr 11, 2015 at 23:06

answered Apr 11, 2015 at 22:28

Messa

25.4k10 gold badges77 silver badges101 bronze badges

3 Comments

JuniorCompressor Over a year ago

Maybe check_positive(**kwargs) for check_positive(width=width, height=height, seed=seed)?

JuniorCompressor Over a year ago

I already upvoted ;) I think it has other useful uses like check_positive(area=width*height). Also, for completeness you may say something for testing.

Queue Overflow Over a year ago

Thanks! This solution is more familiar to me. What do you think about a nesting function check_positive in a function procedural_noise if I will use that function once?

aldeb · Accepted Answer · 2015-04-11 22:31:09Z

2

Regarding the first question, using inspect module:

import inspect

def procedural_noise(width, height, seed):
    frame = inspect.currentframe()
    args, _, _, values = inspect.getargvalues(frame)

    for name in args:
        if values[name] < 0:
             raise ValueError(name + " should be positive")

procedural_noise(3, -66, 2)

Output:

Traceback (most recent call last):
  File "C:\Users\Sam\Desktop\hack.py", line 10, in <module>
    procedural_noise(3, -6, 2)
  File "C:\Users\Sam\Desktop\hack.py", line 8, in procedural_noise
    raise ValueError(name + " should be positive")
ValueError: height should be positive

Otherwise, you could also use dictionary packing this way:

def procedural_noise(**params):
    for name in params.keys():
        if params[name] < 0:
             raise ValueError(name + " should be positive")

procedural_noise(width=3, height=6, seed=-2)

Output:

Traceback (most recent call last):
  File "...\hack.py", line 6, in <module>
    procedural_noise(width=3, height=6, seed=-2)
  File "...\hack.py", line 4, in procedural_noise
    raise ValueError(name + " should be positive")
ValueError: seed should be positive

edited Apr 11, 2015 at 22:31

answered Apr 11, 2015 at 22:11

aldeb

6,8567 gold badges28 silver badges49 bronze badges

4 Comments

MattDMo Over a year ago

However, this requires named parameters when the function is called - something the OP may not want.

MattDMo Over a year ago

That's certainly one way to go about it. Here, have an upvote.

Queue Overflow Over a year ago

Thank you very much. I enjoyed the last your solution with dictionary_packing, but it have one disadvantage: the programmer, that will use this function, will not see a hint with list of parameters of the function in his IDE.

aldeb Over a year ago

Glad I could help :) You're right. That's not the best solution for your problem

Vader · Accepted Answer · 2015-04-11 22:47:38Z

1

Try something like

def procedural_noise(width, height, seed):
    for key,val in locals().items():
        if val < 0:
             raise ValueError(key + " should be positive")

edited Apr 11, 2015 at 22:47

answered Apr 11, 2015 at 22:34

Vader

3,89325 silver badges42 bronze badges

1 Comment

Queue Overflow Over a year ago

Thanks! There is in our country verb: "brevity is the sister of talent" :)

André Laszlo · Accepted Answer · 2015-04-11 22:48:53Z

1

If you find yourself doing this a lot, maybe a decorator will make your code more readable:

def assert_positive(f):
    def wrapper(*args, **kwargs):
        for i, v in enumerate(args):
            if v < 0:
                raise ValueError('The parameter at position %d should be >= 0' % i)
        for k, v in kwargs.items():
            if v < 0:
                raise ValueError('The parameter %s should be >= 0' % k)
        return f(*args, **kwargs)
    return wrapper

Then you could just declare your function like this:

@assert_positive
def procedural_noise(width, height, seed=0):
    ...

It would raise exceptions like this:

>>> procedural_noise(0,-1,0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "argument_checking.py", line 5, in wrapper
    raise ValueError('The parameter at position %d should be >= 0' % i)
ValueError: The parameter at position 1 should be >= 0
>>> procedural_noise(0,0,seed=-1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "argument_checking.py", line 8, in wrapper
    raise ValueError('The parameter %s should be >= 0' % k)
ValueError: The parameter seed should be >= 0

The pythonic way is usually to not check your arguments too much, but there are counterexamples. Examples of builtins that do completely different things:

range(-1) - Looks funny, but is fine, returns []
time.sleep(-1) - Crashes with an IOError: [Errno 22] Invalid argument which I think is just Python's way of saying that a system call returned an error. Maybe I'm lucky that system calls do argument checking...
chr(-1) - ValueError: chr() arg not in range(256)

edited Apr 11, 2015 at 22:48

answered Apr 11, 2015 at 22:42

André Laszlo

15.6k3 gold badges68 silver badges83 bronze badges

1 Comment

Queue Overflow Over a year ago

Thanks. Could you advice some a stuff to read about checking arguments in Python? It is very interesting theme, because I know, that type checking is not approval approach, but there are plenty of situations where is a using variable with incorrect type will crash the work of function.

Dan · Accepted Answer · 2015-04-11 22:22:15Z

0

You can use the all builtin -- all takes an iterable and returns True if all of the items in the iterable are truthy. So for your example, since all positive numbers are truthy, you could write:

def procedural_noise(width, height, seed):
  if all((width, height, seed)):
    # Do stuff
  else:
    raise ValueError("You biffed it.")

Note the double parens -- that's because I'm passing the tuple (width, height, seed) to all -- I can't just write all(width, height, seed), because all expects one iterable argument, like a tuple.

With respect to the testing question, supposing that the name of the module in which procedural_noise is defined is named foo, then in your test file you can use assertRaises with the with keyword, as follows:

import foo
import unittest

class Test(unittest.TestCase):
  def test_positive(self):
    with self.assertRaises(ValueError):
      foo.procedural_noise(-10, -10, 187)

Read more about it at the unittest docs.

edited Apr 11, 2015 at 22:22

answered Apr 11, 2015 at 22:12

Dan

6114 silver badges9 bronze badges

4 Comments

Anshul Goyal Over a year ago

This is simply incorrect. Your code will raise an Error only if all three inputs are 0. Even otherwise, this is clearly not what the OP is looking for, he explicitly states he wants to provide specific exception messages.

Dan Over a year ago

You're right, I had originally written the example as if all((args)): do stuff. I've believe I've fixed it -- if you place the code in a conditional it should have the expected behavior.

Queue Overflow Over a year ago

It is not quite suit for me, but anyway thanks for answer. As for testing question, I wrote, that my function is in a test class, so I have a code, that look a like your code. I asked, is it correct to use assertRaises and ValueError in my situation?

Dan Over a year ago

Yes, it is correct to use assertRaises to check that an error was raised. You might want to try checking each argument individually in the test just to be sure that your code catches all cases, that would involve several assertRaises statements.

Collectives™ on Stack Overflow

Check parameters of a function in a Python

6 Answers 6

1 Comment

3 Comments

4 Comments

1 Comment

1 Comment

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

1 Comment

3 Comments

4 Comments

1 Comment

1 Comment

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related