CODEWITHSUNDEEP
pythoncoding-style
dbr
dbr

Reputation: 169563

How to indent Python list-comprehensions?

List comprehensions can be useful in certain situations, but they can also be rather horrible to read.. As a slightly exaggerated example, how would you indent the following?

allUuids = [x.id for x in self.db.query(schema.allPostsUuid).execute(timeout = 20) if x.type == "post" and x.deleted is not False]

Upvotes: 77

Views: 32198

Answers (8)

Connor Kress
Connor Kress

Reputation: 11

    allUuids = [
        x.id
        for x in self.db.query(schema.allPostsUuid).execute(timeout = 20)
        if x.type == "post"
            and x.deleted
    ]

Upvotes: 1

Henco
Henco

Reputation: 59

If you're set on a comprehension orestis's answer is good.

For more complex comprehensions like that I'd suggest using a generator with yield:

allUuids = list(self.get_all_uuids())


def get_all_uuids(self):
    for x in self.db.query(schema.allPostsUuid).execute(timeout = 20):
        if x.type == "post" and x.deleted is not False:
            yield x.id

Upvotes: 3

Bite code
Bite code

Reputation: 596703

You should not use a list comprehension for that.

List comprehensions are an awesome feature, but they are meant to be shortcuts, not regular code.

For such a long snippet, you should use ordinary blocs :

allUuids = []
for x in self.db.query(schema.allPostsUuid).execute(timeout = 20) :
    if x.type == "post" and x.deleted is not False :
        allUuids.append(x.id)

Exactly the same behavior, much more readable. Guido would be proud of you :-)

Upvotes: 3

jfs
jfs

Reputation: 414199

allUuids = [x.id 
            for x in self.db.query(schema.allPostsUuid).execute(timeout = 20) 
            if x.type == "post" and x.deleted is not False]

Upvotes: 8

guzzloid
guzzloid

Reputation:

Where I work, our coding guidelines would have us do something like this:

all_posts_uuid_query = self.db.query(schema.allPostsUuid)
all_posts_uuid_list = all_posts_uuid_query.execute(timeout=20)
all_uuid_list = [
    x.id 
    for x in all_posts_uuid_list 
    if (
        x.type == "post" 
        and 
        not x.deleted  # <-- if you don't care about NULLs / None
    )
]

Upvotes: 58

orestis
orestis

Reputation: 17180

It depends on how long they are. I tend to structure them like so:

[x.id for x
 in self.db.query(schema.allPostsUuid).execute(timeout=20)
 if x.type == 'post' 
    and x.deleted is not False
    and ...
    and ...]

That way every expression has its own line.

If any line becomes too big I like to extract it out in a lambda or expression:

transform = lambda x: x.id
results = self.db.query(schema.allPostsUuid).execute(timeout=20)
condition = lambda x: x.deleted is not False and ... and ...
[transform(x) for x in results if condition(x)]

And then if a lambda becomes too long it gets promoted to a function.

Upvotes: 80

Ali Afshar
Ali Afshar

Reputation: 41643

For me that's too much. Maybe it's just a terrible example, since "type" and "deleted" would clearly be part of the db query.

I tend to think that if a list comprehension spans multiple lines it probably shouldn't be a list comprehension. Having said that, I usually just split the thing at "if" like other people have and will answer here.

Upvotes: 6

Eli Bendersky
Eli Bendersky

Reputation: 273436

How about:

allUuids = [x.id for x in self.db.query(schema.allPostsUuid).execute(timeout = 20) 
                   if (x.type == "post" and x.deleted is not False)]

Generally, long lines can be avoided by pre-computing subexpressions into variables, which might add a minuscule performance cost:

query_ids = self.db.query(schema.allPostsUuid).execute(timeout = 20)
allUuids = [x.id for x in query_ids
                   if (x.type == "post" and x.deleted is not False)]

By the way, isn't 'is not False' kind-of superfluous ? Are you worried about differentiating between None and False ? Because otherwise, it suffices to leave the condition as only: if (x.type == "post" and x.deleted)

Upvotes: 1

Related Questions