List comprehension for testing **params

[Cantabile]

Hi,
I'm writing a small mail library for my own use, and at the time I'm
testing parameters like this:

class Mail(object):
def __init__(self, smtp, login, **params)
blah
blah
required = ['Subject', 'From', 'To', 'msg']
for i in required:
if not i in params.keys():
print "Error, \'%s\' argument is missing" %i
exit(1)
...

md = {'Subject' : 'Test', 'From' :'Me', 'To' :['You', 'Them'], 'msg'
:my.txt"}

m = Mail('smtp.myprovider.com', ["mylogin", "mypasswd"], **md)



I'd like to do something like that instead of the 'for' loop in __init__:

assert[key for key in required if key in params.keys()]

but it doesn't work. It doesn't find anythin wrong if remove, say msg,
from **md. I thought it should because I believed that this list
comprehension would check that every keyword in required would have a
match in params.keys.

Could you explain why it doesn't work and do you have any idea of how it
could work ?

Thanks in advance
Cheers,
Cantabile

 

[Tim Chase]

assert[key for key in required if key in params.keys()]
....

Could you explain why it doesn't work and do you have any idea of how it
could work ?

Well, here, if any of the items are found, you get a list that is
non-False'ish, so the assert passes.

It sounds like you want all() (available as of 2.5)

assert all(key in params for key in required)

This is modulo any key normalization for "From" vs. "FROM" vs.
"from", etc; but should be enough to pass your initial requirements.

-tkc

 

[Ian Kelly]

I'd like to do something like that instead of the 'for' loop in __init__:

assert[key for key in required if key in params.keys()]

A list evaluates as true if it is not empty. As long as at least one
of the required parameters is present, the list will not be empty and
will evaluate as true, and the assertion will pass. Try this instead:

assert all(key in params.keys() for key in required)

By the way, using assertions for input checking is generally not
considered good practice. As soon as Python is invoked with the -O
option, your input testing is tossed out the window. Assertions are
good to use for internal logic testing, to avoid unexpected state.
For input checking, use an explicit if test and exception:

if any(key not in params.keys() for key in required):
raise ValueError("Missing required keyword argument")

Additionally, if the arguments are required, then they probably have
no business being wrapped up in **params in the first place. Why not
just define your method like:

def __init__(self, smtp, login, subject, from, to, msg):
# ...

And then Python will do all the work of requiring them to be present for you.

 

[Steven D'Aprano]

Hi,
I'm writing a small mail library for my own use, and at the time I'm
testing parameters like this:

class Mail(object):
def __init__(self, smtp, login, **params)
blah
blah
required = ['Subject', 'From', 'To', 'msg'] for i in required:
if not i in params.keys():
print "Error, \'%s\' argument is missing" %i exit(1)
...

Eeek! Don't do that. Seriously dude, that is Evil. Why should a simple
missing argument to a class *kill the entire application*????

How horrible would it be if you called "chr()" and instead of raising a
TypeError, Python shut down? Don't do that. Raise a proper exception.
Then you have the choice to either catch the exception and continue, or
let it propagate as appropriate.


But you shouldn't even be manually checking for required arguments. Just
use real arguments:

def __init__(self, smpt, login, Subject, From, To, msg, **params):
blah


See how easy it is? Problem solved. If you don't provide an argument,
Python gives you an exception for free. Best of all, you can use
inspection to see what arguments are required, and help(Mail) shows you
what arguments are needed.

You can still use **md as below, and Python will automatically do
everything you're manually trying to do (and failing). Don't fight the
language, even when you win, you lose.

md = {'Subject' : 'Test', 'From' :'Me', 'To' :['You', 'Them'], 'msg'
:my.txt"}

m = Mail('smtp.myprovider.com', ["mylogin", "mypasswd"], **md)

Conventionally, a tuple ("mylogin", "mypasswd") would express the intent
better than a list.

I'd like to do something like that instead of the 'for' loop in
__init__:

assert[key for key in required if key in params.keys()]

but it doesn't work. It doesn't find anythin wrong if remove, say msg,
from **md. I thought it should because I believed that this list
comprehension would check that every keyword in required would have a
match in params.keys.

No, you have three problems there.

The first problem is that only the empty list is falsey:

assert [] # this raises an exception
assert ["Subject", "From"] # this doesn't, even though "To" and "msg"
are missing.

You could say:

assert all([key in params for key in required])

but that leaves you with the next two problems:

2) Fixing the assert still leaves you with the wrong exception. You
wouldn't raise a ZeroDivisionError, or a UnicodeDecodeError, or an IOError
would you? No of course not. So why are you suggesting raising an
AssertionError? That is completely inappropriate, the right exception to
use is TypeError. Don't be lazy and treat assert as a quick and easy way
to get exceptions for free.

3) Asserts are not guaranteed to run. Never, never, never use assert to
test function arguments supplied by the caller, because you have no
control over whether or not the asserts will even be called.

The whole point of assertions is that the caller can disable them for
speed. Asserts should be used for testing your code's internal logic, not
user-provided arguments. Or for testing things which "can't possibly
fail". But of course, since what can't go wrong does, you sometimes still
want to test things.

If you've ever written something like:

if condition:
process()
else:
# this cannot happen, but just in case!
print "something unexpected went wrong!"


that's a perfect candidate for an assertion:

assert condition, "something unexpected went wrong!"
process()

 

[Terry Reedy]

I'd like to do something like that instead of the 'for' loop in __init__:

assert[key for key in required if key in params.keys()]

A list evaluates as true if it is not empty. As long as at least one
of the required parameters is present, the list will not be empty and
will evaluate as true, and the assertion will pass. Try this instead:

assert all(key in params.keys() for key in required)

By the way, using assertions for input checking is generally not
considered good practice. As soon as Python is invoked with the -O
option, your input testing is tossed out the window. Assertions are
good to use for internal logic testing, to avoid unexpected state.
For input checking, use an explicit if test and exception:

if any(key not in params.keys() for key in required):
raise ValueError("Missing required keyword argument")

Additionally, if the arguments are required, then they probably have
no business being wrapped up in **params in the first place. Why not
just define your method like:

def __init__(self, smtp, login, subject, from, to, msg):

or if you want them to be identified by keyword only (since 7 positional
args is a bit much)

def __init__(self, smtp, login, *, subject, from, to, msg):

(I forget when this feature was added)

 

[Steven D'Aprano]

or if you want them to be identified by keyword only (since 7 positional
args is a bit much)

def __init__(self, smtp, login, *, subject, from, to, msg):

(I forget when this feature was added)

It's a Python 3 feature.

 

[Cantabile]

Thanks everyone for your answers. That's much clearer now.
I see that I was somehow fighting python instead of using it. Lesson
learned (for the time being at least)
I'll probably get back with more questions...
Cheers,
Cantabile

 

[Tim Chase]

but that leaves you with the next two problems:

2) Fixing the assert still leaves you with the wrong exception. You
wouldn't raise a ZeroDivisionError, or a UnicodeDecodeError, or an IOError
would you? No of course not. So why are you suggesting raising an
AssertionError? That is completely inappropriate, the right exception to
use is TypeError. Don't be lazy and treat assert as a quick and easy way
to get exceptions for free.

I'd say that it depends on whether the dictionary/kwargs gets
populated from user-supplied data (whether mail, a file, direct
input, etc), or whether it's coder-supplied.

I like to think of "assert" as a "hey, you bone-headed programmer,
you did something that violated conditions for using this code
properly!" and as such, entirely appropriate to use in the
coder-supplied case. This rolls into...

3) Asserts are not guaranteed to run. Never, never, never use assert to
test function arguments supplied by the caller, because you have no
control over whether or not the asserts will even be called.

where, once the programmer is satisfied that the code runs, meeting
all the appropriate tests/assertions, the assertions can then be
turned off (if one really cares that much; can't say I've done it
more than a couple times, mostly out of experimentation/testing to
see if it made any grave difference in performance). I think you
intend this in your

Or for testing things which "can't possibly fail". But of
course, since what can't go wrong does, you sometimes still want
to test things.

-tkc

 

[Steven D'Aprano]

I'd say that it depends on whether the dictionary/kwargs gets populated
from user-supplied data (whether mail, a file, direct input, etc), or
whether it's coder-supplied.

No, it doesn't matter if it is end-user supplied or coder-supplied. As
usual, look at the standard library and Python builtins for best
practices:

py> len(42) # do we get an AssertionError?
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: object of type 'int' has no len()

I like to think of "assert" as a "hey, you bone-headed programmer, you
did something that violated conditions for using this code properly!"

You mean like passing a list to ord()? Or trying to add a number and a
string?

Raise the appropriate exception for the error: TypeError for type errors
and missing function parameters, ValueErrors for objects of the right
type but a bad argument, etc. Assertion errors are for asserting facts
about internal program logic. "I assert that such-and-such is a fact",
rather than "I require that such-and-such is a fact".

For example, this might be a reasonable (although trivial) use of assert:

def spam(n):
if isinstance(n, int) and n > 0:
result = ' '.join(["spam"]*n)
assert result, "expected non-empty string but got empty"
return result
else:
raise SpamError("No spam for you!!!")

This gives a measure of protection against logic errors such as writing
n >= 0.

The presence of the assert is partly documentation and partly error
checking, but the important thing is that it checks things which depend
on the internal logic of the code itself, not on the input to the code.
Given that you have already explicitly tested that n > 0, then the
logical consequence is that "spam"*n will be non-empty. Hence an
assertion "I assert that the string is not empty", rather than an overt
test "Is the string empty?"

Now, I accept that sometimes it is hard to draw a line between "internal
program logic" and external input. E.g. if one function generates a value
x, then calls another function with x as argument, the state of x is an
internal detail to the first function and an external input to the other
function. The rules I use are:

* If the argument is a public parameter to a public function,
then you must not use assert to validate it;

* If the argument is a private parameter, or if the function
is a private function, then you may use assert to validate
it (but probably shouldn't);

* You can use assert to check conditions of function internal
variables (locals that you control);

* Once you cross the boundary to another function, your objects
should be treated as external again.

E.g. I might have:

def foo(a, b):
# foo, a and b are public, so don't use assert
if not (isinstance(a, int) and isinstance(b, int)):
raise TypeError
x = abs(a) + abs(b)
# x is purely internal, so okay to assert
assert x >= 0, "Hey McFly, you screwed up!"
return bar(x)

def bar(x):
if x <= 0:
raise ValueError("expected positive x")
...

This now guards against something other than foo calling bar.

 

[Ulrich Eckhardt]

Am 11.11.2012 23:24, schrieb Cantabile:

I'm writing a small mail library for my own use, and at the time I'm
testing parameters like this:

Let's ignore the facts that there is an existing mail library, that you
should use real parameters if they are required and that exit() is
completely inappropriate. Others explained why sufficiently.

[slightly shortened]

def function(**params)
required = ['Subject', 'From', 'To', 'msg']
for i in required:
if not i in params.keys():
print "Error, \'%s\' argument is missing" %i

Let's start with the last line: If you use "Error, missing {!r}
argument".format(i), you get the quotes automatically, plus possibly
escaped unicode characters and newlines, although it's not necessary.
Also, I would "from __future__ import print_function" in any new code,
to ease upgrading to Python 3.

Now, concerning the test whether the required parameters are passed to
the function, you can use "if i in params", which is slightly shorter
and IMHO similarly expressive. Also, it doesn't repeatedly create a
list, checks for a single element inside that list and then throws the
list away again. Further, you don't even need a list for the parameters,
since order doesn't matter and duplicates shouldn't exist, so I would
use a set instead:

required = {'Subject', 'From', 'To', 'msg'}

The advantage is slightly faster lookup (completely irrelevant for this
small number of elements) but also that it makes the intention a bit
clearer to people that know what a set is.

In a second step, you compute the intersection between the set of
required arguments and the set of supplied arguments and verify that all
are present:

if required.intersection(params.keys()) != required:
missing = required - set(params.keys())
raise Exception("missing arguments {}".format(
', '.join(missing)))


Happy hacking!

Uli

 

[Cantabile]

Wow, lots of things I had never heard of in your posts.
I guess I need to do some homework...

Cantabile