List comprehension for testing **params

Discussion in 'Python' started by Cantabile, Nov 11, 2012.

  1. Cantabile

    Cantabile Guest

    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
     
    Cantabile, Nov 11, 2012
    #1
    1. Advertising

  2. Cantabile

    Tim Chase Guest

    > 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
     
    Tim Chase, Nov 11, 2012
    #2
    1. Advertising

  3. Cantabile

    Ian Kelly Guest

    On Sun, Nov 11, 2012 at 3:24 PM, Cantabile <> wrote:
    > 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.
     
    Ian Kelly, Nov 11, 2012
    #3
  4. On Sun, 11 Nov 2012 23:24:14 +0100, Cantabile wrote:

    > 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()



    --
    Steven
     
    Steven D'Aprano, Nov 11, 2012
    #4
  5. Cantabile

    Terry Reedy Guest

    On 11/11/2012 5:56 PM, Ian Kelly wrote:
    > On Sun, Nov 11, 2012 at 3:24 PM, Cantabile <> wrote:
    >> 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)


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



    --
    Terry Jan Reedy
     
    Terry Reedy, Nov 11, 2012
    #5
  6. On Sun, 11 Nov 2012 18:37:05 -0500, Terry Reedy wrote:

    > 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.



    --
    Steven
     
    Steven D'Aprano, Nov 11, 2012
    #6
  7. Cantabile

    Cantabile Guest

    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
     
    Cantabile, Nov 11, 2012
    #7
  8. Cantabile

    Tim Chase Guest

    On 11/11/12 17:18, Steven D'Aprano wrote:
    > 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
     
    Tim Chase, Nov 12, 2012
    #8
  9. On Sun, 11 Nov 2012 18:21:32 -0600, Tim Chase wrote:

    > On 11/11/12 17:18, Steven D'Aprano wrote:
    >> 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.


    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.



    --
    Steven
     
    Steven D'Aprano, Nov 12, 2012
    #9
  10. 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
     
    Ulrich Eckhardt, Nov 12, 2012
    #10
  11. Cantabile

    Cantabile Guest

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

    Cantabile
     
    Cantabile, Nov 12, 2012
    #11
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Shane Geiger
    Replies:
    4
    Views:
    399
    bullockbefriending bard
    Mar 25, 2007
  2. Debajit Adhikary
    Replies:
    17
    Views:
    702
    Debajit Adhikary
    Oct 18, 2007
  3. Vedran Furac(
    Replies:
    4
    Views:
    340
    Marc 'BlackJack' Rintsch
    Dec 19, 2008
  4. Barry
    Replies:
    9
    Views:
    470
    Ara.T.Howard
    Sep 15, 2005
  5. Ashley Moran

    I can't get hash list params working

    Ashley Moran, Apr 2, 2006, in forum: Ruby
    Replies:
    1
    Views:
    110
Loading...

Share This Page