Anti-Patterns in Python Programming

[tribaal]

The more frequent and dangerous pitfalls are, in my humble opinion:

- Bare except: statements (that catches everything, even Ctrl-C)

- Mutables as default function/method arguments

- Wildcard imports!

[SEJeff]

Couldn't agree more! One of my all time new python interview questions gets a surprisingly large number of developers.

Given a function like:

    def append_one(l=[]):

        l.append(1)

        return l

What does this return each time?

    >>> append_one()

    >>> append_one()

    >>> append_one()

[scott_w]

Possibly the most interesting anti-pattern I saw was:

a_list_of_words = "my list of words".split(" ")

I never enquired why, since there were bigger issues in the code e.g. "unit testing" by running the code, taking the result and putting it as the check value. By running repr(value), copying out the string then comparing self.assertEqual(repr(value), '[<Object1: unicode_value>, ...]')

[cabalamat]

> Mutables as default function/method arguments

It would really make sense to change the semantics of Python to fix this issue.

[LyndsySimon]

Wildcard imports are OK, if and only if the module is specifically designed with this usage in mind.

Example: nose.tools

[geertj]

Agreed on all counts. However I do find myself using mutables as default arguments sometimes because the generated documentation is clearer.

For example, this is a real method in one of my projects:

  def listen(self, address, ssl=False, ssl_args={}):
      pass

I like the way this turns up in the docs because it's immediately clear that ssl_args needs to be a dict. Otherwise I have to describe it in words.

[peter-row]

Also, not knowing what scope a variable has when creating closures.

[doctoboggan]

I agree that bare excepts are bad, however they do not catch Ctrl-C. If you _do_ want to catch Ctrl-C you have to except a KeyboardInterrupt explicitly.

[shadowmint]

Mmm... you should always use 'if x is not None:' imo.

It's very common for libraries to make values evaluate to False, and very easy to get bugs if you just lazily test with 'if x'.

Sqlalchemy springs to mind immediately as one of the common ones where using any() and if x: is a reeeeeallly bad idea; there are plenty of others.

I'm pretty skpetical about modifying your coding behavior based on what libraries you happen to be currently using.

'If x' isn't your friend.

[tomp]

Especially taking into account the more bizarre bugs (features?) of python:

    (bool(datetime.time(0)), bool(datetime.time(1))) == (False, True)

I always consider `if x:` a bug, unless x can only be a boolean. Furthermore, it seriously hinders readability and clarity of the code.

[glimcat]

    "Explicit is better than implicit."

If you mean is not None, you should say is not None.

It's fast and readable and there are no "just be aware that" disclaimers to tack on afterwards.

[LyndsySimon]

It depends.

If you're checking to see if that value is None, then yes - you should check that.

If you're merely checking if the value is truthy, then using "if x:" is completely legitimate.

[msvalkon]

Check out Raymond Hettingers Transforming Code into Beautiful, Idiomatic Python talk on youtube [1].

Great talk on avoiding some of the common pitfalls new python developers step in. Exposes some nice language features.

[1]: https://www.youtube.com/watch?v=OSGv2VnC0go

[gshubert17]

And the slides are here:

https://speakerdeck.com/pyconslides/transforming-code-into-b...

[sophacles]

The only thing I disagree with is "use nested comprehensions" thing. In my mind: x = [letter for word in words for letter in word]

is inside-out or backwards or backwards. I want the nested for being the less specific case:

   x = [letter for letter in word for word in words]

makes more sense in my mind.

(It's also my first answer to the "what're some warts in the your language of choice).

[njharman]

I'm in the camp that if your list comp needs more than one for clause, it's complicated enough to be broken out into actual for loop.

[jules]

Even clearer:

    x = [for word in words: for letter in word: letter]

This also has the advantage of being readable left to right without encountering any unbound identifiers like all other constructs in Python.

[cuu508]

> write a list comprehension (...) code just looks a lot cleaner and what you're doing is clearer.

I know how to use list comprehensions, but often avoid using them and use the standard for loops. List comprehensions look nice and clean for small examples, but they can easily get long and become mentally hard to parse. I would rather go for three 30 character lines instead of one 90 character line.

[bkeroack]

Depends on how you want to program: imperative vs functional.

Personally I think list comprehensions are the most beautiful part of Python, though sometimes I use map() when I'm trying to be explicitly functional (I realize it's allegedly slower, etc).

Generally I think list comprehensions are cleaner and allow you to write purer functions with fewer mutable variables. I disagree that deeply nested for loops are necessarily more readable.

[meowface]

I'm a bit torn on it.

In a case where you need to do a lot of nested appends, I've found that even a long list comprehension can be easier to read. You just have to be sure to properly indent it and break it up into multiple lines. My rule is that every extra `for` starts a new line, and sometimes moving the predicate to its own line when it's too long, too.

[thinkpad20]

I agree. I try to use list comprehensions when I can, but the fact is that as soon as you have to do any sort of logic in your map, it quickly gets unwieldy. I'd much rather use a functional style in many of these cases, but python's syntax and crummy lambdas (as compared to what would be natural to do in say JavaScript or Ruby), means that in the majority of cases I find myself using an imperative style.

[moretti]

I always struggle to understand why a list comprehension

  alist = [foo(word) for word in words]

is considered more Pythonic than map

  alist = map(foo, words)

[dagw]

List comprehensions are more flexible and easier to read in the non-trivial case. Sure in the trivial case you show a map might be considered neater, but just adding a filter is enough to make the list comprehension more readable in my mind. Python's lambda syntax also makes using maps and filters quite ugly.

Compare:

   alist = [x**2 for x in mylist if x%3==0]

to

   alist = map(lambda x: x**2,filter(lambda x: x%3==0, mylist)

Plus python also has set comprehension and dict comprehension, which share essentially the same syntax.

[blossoms]

    alist = [foo(word) for word in words if word.startswith('a')]
    alist = map(foo, filter(lambda word: word.startswith('a'), words))

Which reads better?

[metaphorm]

you can consider a list comprehension to be a sort of literal representation of the result of map. I think literals have a benefit for code readability and should be used when feasible (i.e. the literal is compact enough).

the other reason its considered more idiomatic in Python is just because the compiler does a better job of parsing and optimizing list comprehensions.

[XorNot]

You use list comprehensions in other places you wouldn't use map. The difference in text size as you posted is minimal, but the list comprehension - once you're used to reading them - tells you exactly what's going on.

Where as map could be anything. It could be redefined for all you'd know.

[a3n]

Don't know if this is why, but the list comprehension takes an expression at the "foo(word)" location, and is therefore more general than map, which requires a function. The comprehension in that case is simpler.

  words = ['w1', 'w2', 'w3']

  [word[1] for word in words]

  ['1', '2', '3']
  
  map(lambda x: x[1], words)

  ['1', '2', '3']

I like looking at the list comprehension better. The use of lambda looks forced in this case. I also imagine there's a penalty for calling the (anonymous) function in map.

[TheLoneWolfling]

Those are not equivalent.

You cannot index a map object, for example.

[ufo]

I think the main reason is the lambda syntax. List comprehensions also let you do filter and nested loops.

[ggchappell]

This is a nice little article, but I wonder about some of the design decisions. In particular:

> The simplifications employed (for example, ignoring generators and the power of itertools when talking about iteration) reflect its intended audience.

Are generators really that hard? (Not a rhetorical question!)

The article mentions problems resulting from the creation of a temporary list based on a large initial list. So, why not just replace a list comprehension "[ ... ]" with a generator expression "( ... )"? Result: minimal storage requirements, and no computation of values later than those that are actually used.

And then there is itertools. This package might seem a bit unintuitive to those who have only programmed in "C". But I think the solution to that is to give examples of how itertools can be used to create simple, readable, efficient code.

[omegote]

Point 3 of the iteration part is not good advice. With [1:] you're making a copy of the list just to iterate over it...

[omaranto]

You're right. I still wouldn't recommend looping over indices, but rather using itertools.islice(xs, 1, None) instead of xs[1:].

[zo1]

You're right... But at least it's a shallow copy.

[msl09]

I find that the testing for empty is a bit misguided if you want to be rigorous with types. for instance:

     >>>def isempty(l):
     >>>    return not bool(l)
     >>>isempty([])
     True
     >>>isempty(None)
     True

If embedded within your program logic this kind of pattern can waste precious time with debugging. You can catch your errors much more quickly if you are explicit with your comparisons.

[elandybarr]

Failing to use join is a big one.

I have seen countless instances of people writing the logic to output commas in between items (like for CSV export) that they want to concatenate into a string.

    header_line = ','.join( header for header in headers )
    csv_line    = ','.join( str(dataset[key]) for key in dataset.keys() )

Example for a case of a dictionary mapping a string to a bunch of numbers.

[marcinw]

Proper Python would use the csv module for this operation, as your CSV export would break if `header` or `dataset[key]` contains a comma.

[cpenner461]

Any reason not to do the first one more compactly?

     ','.join(headers)

[rcfox]

You should just use dataset.values() (or itervalues if you're using Python 2) instead of iterating over the keys, and then looking them up.

[yeukhon]

> PEP 8 is the universal style guide for Python code.

> If you aren't following it, you should have good reasons beyond "I just don't like the way that looks."

Core dev and Guido have said many times PEP 8 are not holy.

See https://mail.python.org/pipermail/python-dev/2010-November/1...

In essence, a "stupid reason" like "I don't like it" is a valid reason not to adopt PEP 8.

In fact, I don't like the PEP 8 recommendation on docstring. I like Google's docstring (aka napoleon in Sphinx contrib-module).

http://sphinxcontrib-napoleon.readthedocs.org/en/latest/exam...

[orf]

Interesting read, a couple of things I noticed though:

1. In "Checking for contents in linear time" both examples are the same. Perhaps remove the list entirely in the second example

2. Itertools.islice helps if you need to slice a list with a bajillion elements

[gr3yh47]

# Avoid this

lyrics_list = ['her', 'name', 'is', 'rio']

words = make_wordlist() # Pretend this returns many words that we want to test

for word in words:

    if word in lyrics_list: # Linear time

        print word, "is in the lyrics"

# Do this

lyrics_list = ['her', 'name', 'is', 'rio']

lyrics_set = set(lyrics_list) # Linear time set construction

words = make_wordlist() # Pretend this returns many words that we want to test

for word in words:

    if word in lyrics_list: # Constant time

        print word, "is in the lyrics"

the second example should read ... if word in lyrics_set: ...

[wodenokoto]

I use python for datamining, and most of my work is done exploring data in iPython.

> First, don't set any values in the outer scope that > aren't IN_ALL_CAPS. Things like parsing arguments are > best delegated to a function named main, so that any > internal variables in that function do not live in the > outer scope.

How do I inspect variables in my main function after I get unexpected results? I always have my main logic live in the outer scope because I often inspect variables "after the fact" in iPython.

How should I be doing this?

[parham]

Instead of this:

    # Do this  
    lyrics_set = set(lyrics_list) # Linear time set construction  
    words = make_wordlist()  
    for word in words:  
        if word in lyrics_set: # Constant time  
            print word, "is in the lyrics"

You could do this:

    lyrics_set = set(lyrics_list)  
    words = set(make_wordlist())  
    matched_words = list(lyrics_set & words)  
    for word in matched_words:  
        print word, "is in the lyrics"

[LyndsySimon]

I can't think of a single case where using sentinel values is necessary or appropriate in Python.

Generally speaking, one should just return from within the loop.

[u124556]

> Consider using xrange in this case. Is xrange still a thing? doesn't range use a generator instead of creating a list nowadays?

[omaranto]

Well, I'd say no, not in "Python".

It seems to me that when people say "Python" they still mostly mean Python 2, where range returns a list and xrange a generator. In Python 3 there is no xrange and range returns a generator, but I think people still most often call that language "Python 3", not "Python".

[minopret]

Yes, I'd revise as "Consider using Python 3 in this case." This is a chief reason why I now avoid Python 2. Python 3 is more than five years old. Where we have discretion to choose Python 3, it's time to exercise that discretion. Not because 3 is greater than 2. Because Python 2 has prominent pains that are healed in Python 3.

[me_myself_and_I]

I don't think there us such thing as a pattern per language, unless the language is really unique. IMO what does exists is Language Bad Practices, which are actually tied to the language itself.

An (anti)pattern is something abstract and can be applied to any other similar language.

[yoo-interested]

Speaking of find\_item, is the `for..else` loop (which can be used to write find\_item in another way) considered Pythonic? I personally like `for..else` loops but I don't know where the consensus is at.

[metaphorm]

an anti-pattern is a design pattern gone bad. these aren't anti-patterns. they're just noobie mistakes and non-idiomatic expressions.