1

originally I had this awful piece of code with two for i in. To sum up, the idea is to get a list of AMI's ID of some AMIS with a particular tag. To do so, I iterate over list (wrong!) and I get a list of IDS.

def aws_get_images_exceptions(connection, exceptions):
 list_exceptions = []
    for ex in exceptions:
        images = connection.get_all_images(filters={ "tag-key":"Name","tag-value":ex})
        for ami in images:
            list_exceptions.append(ami.id)
    return list_exceptions

Over each iteration I get the complete list of "exceptions" based on the fixed tag Role that I provide to function and then I append the right AMIS.

Ok, I was able to remove the second for i in with the next code:

def aws_get_images_exceptions(connection, exceptions):
for ex in exceptions:
        images = connection.get_all_images(filters={ "tag-key":"Name","tag-value":ex})
       list_exceptions = list(map(lambda x: x.id, images)) + list_exceptions
return list_exceptions

I've checked and I get the same amount of AMIS, same IDS, etc. But I don't get the way to do it in a functional way removing the very first for. As you notice I do a manual call to get_all_images because I iterate over a list of "exceptions" which I give to the function.

Do you know how I can remove this first for i in?

Thanks to all!

Rory Daulton
  • 21,934
  • 6
  • 42
  • 50
Rubendob
  • 1,614
  • 3
  • 22
  • 34

3 Answers3

4

you don't need to remove for-loops (especially with lambdas) because it is normal and readable enough, however it will be better to extract images fetching to separate function using generator

def get_images(connection, exceptions):
    for ex in exceptions:
        images = connection.get_all_images(filters={"tag-key": "Name", "tag-value": ex})
        # in Python 3.3+ instead of loop use
        # yield from images
        for image in images:
            yield image

and after that initial function will be

def aws_get_images_exceptions(connection, exceptions):
    images = get_images(connection, exceptions)
    return [ami.id for ami in images]

and i don't think it can be written fully in functional way since it looks like you are making calls to some sort of database (and this is a side effect)

Further reading

about yield keyword and generators

P. S.

get_images can also be re-written without using generator

def get_images(connection, exceptions):
    images = []
    for ex in exceptions:
        images += connection.get_all_images(filters={"tag-key": "Name", "tag-value": ex})
    return images

but generators are truly awesome and you will see how powerful they are.

Azat Ibrakov
  • 9,998
  • 9
  • 38
  • 50
  • Upvoted, because this is a better answer than my own. The combination of using a separate generator and a much simpler list comprehension makes it a winner in terms of Pythonicness. – acdr Jun 07 '17 at 14:55
2

It's certainly possible, simply by nesting fors in your list comprehension:

def aws_get_images_exceptions(connection, exceptions):
    list_exceptions = [ami.id for ex in exceptions for ami in connection.get_all_images(filters={ "tag-key":"Name","tag-value":ex})]
    return list_exceptions

However, that doesn't mean it's good code. I find your original function much easier to read than my own abomination here.

acdr
  • 4,538
  • 2
  • 19
  • 45
  • Well, some college here is particularly aggressive against nested for and this kind of stuff so I was trying to refactor taking advantage of the functional programming :) Was my main objective. I don't know really which is the best or bad way to do it jeje – Rubendob Jun 07 '17 at 14:51
1

Functional style doesn't necessarily mean writing a one-line list comprehension to do everything. The essence of what you want is to iterate over a bunch of streams; you have a list of exceptions, each of which produces a list of images to iterate over. You can use natural Python syntax handle this just as cleanly.

def aws_get_images_exceptions(connection, exceptions):
    for ex in exceptions:
        filters = {"tag-key": "Name", "tag-value": ex}
        for ami in connection.get_all_images(filters=filters):
            yield ami.id

Don't worry about defining the variable filters; temporary variables are often used (via let expressions) even in pure functional languages. Otherwise, you don't have any real side effects (other than the unavoidable database query).

Notice the only significant change I've made to your code is to yield each id, rather than append it to a list that gets returned. The caller can decide whether to create a list from the resulting generator.

ids = list(aws_get_images_exceptions(connection, exceptions))
chepner
  • 497,756
  • 71
  • 530
  • 681
  • Hi, I'm using Python mainly due to scripting purposes but I feel this concept of yield very interesting. I should definitely take a look to it :) – Rubendob Jun 07 '17 at 15:40