Python Anti-Pattern

https://valinsky.me/articles/python-anti-pattern/

by valinsky • 5 years ago

37 25 5 years ago

valinsky.me

EdwardDiego2• 5 years ago

Yep, a renowned Python footgun. Although Lambda container reuse adds a modern twist to it.

Can I ask what editor you were using? Because decent ones should have warned you about mutable default args.

davidatbu6• 5 years ago

Shout out to the amazing Pyright, the fast and feature-complete python LSP that you can use with any editor that supports the LSP protocol!

valinsky2• 5 years ago

I'm using VSCode with different Python specific extensions, like Pylance which uses Pyright, but it didn't warn me about the mutable function parameter. RIP

bjoli3• 5 years ago

About half the bugs I fix is due to unexpected interactions with mutable state.

Now, I am usually the creator of the bugs, and it might be that I completely lacks any intellectual rigour with regards to mutable state, but it is re-assuring that I am bot the only one.

This specific behaviour of python has always struck me as completely idiotic.

davidatbu6• 5 years ago

This specific behaviour of python has always struck me as completely idiotic

I'm curious, what would be your proposed solution? Make a special case exception for when the expression that constitutes a function default is evaluated? What would that exception look like?

neoncontrails• 5 years ago

Possibly something like this? (Code for illustration purposes only, of course -- please no one use this.)

    class AntiAntiPattern:
      2     """A quick and dirty attempt to mock a Python class instance that 'undoes' the antipattern 
      3        described here: 
      4          https://docs.quantifiedcode.com/python-anti-patterns/correctness/mutable_default_value_as_argument.html)"""
      5     __mutable_args__ = {} # TODO: a real implementation would probably use a WeakMap of some sort
      6     def __init__(self, a, b=None, c={}, d=set()):
      7         self.a = a
      8         self.b = "foo" if b is None else b
      9         self.c = c
     10         self.d = "bar" if d is None else d
     11     def __getattr__(self, k):
     12         return self.__mutable_args__[k] if k in self.__mutable_args__ else super().getattr(k)
     13     def __setattr__(self, k, v):
     14         if v in self.__init__.__defaults__:
     15             self.__mutable_args__[k] = v
     16         else:
     17             self.__dict__[k] = v
     18     def __repr__(self):
     19         return json.dumps(dict(self.__dict__, **self.__mutable_args__), default=str)
    >>> t = AntiAntiPattern(4, d = 'hello!')
    >>> t.__mutable_args__
    {'b': None, 'c': {}}
    
    >>> t.__dict__
    {'a': 4, 'd': 'hello!'}
    
    >>> t
    {"a": 4, "d": "hello!", "b": null, "c": {}}

Edit: Oops, fixed a tiny logic bug in __init__. Trying to ween myself off of the 'a = a or "my_a_default"' syntax as instructed by previous commenters in this thread. :)

Edit2: It dawned on me that this doesn't actually resolve the 'gotcha' unless `v` is replaced with `deepcopy(v)` on line 15; otherwise the same exact problem rears its head:

    >>> t = AntiAntiPattern(3, {})
    >>> t.d
    set()
    
    >>> t.d.update([3,4])
    >>> t
    {"a": 3, "b": {}, "c": {}, "d": "{3, 4}"}
    
    >>> t2 = AntiAntiPattern(4)
    >>> t2
    {"a": 4, "b": {}, "c": {}, "d": "{3, 4}"} # aw, heck

In which case I gather `__mutable_args__` is superfluous, and the solution involves simply making sure that class instances avoid pointing to the same memory objects?

bjoli3• 5 years ago

My solution would be to fix it and break backwards compatibility: Any default value is evaluated anew on each function call without the specified value. Then you CAN mutate it to your hearts desire without impacting subsequent invocations. If you want to keep hidden mutable state between function invocations you should use a closure.

    def banana(parts = []):
      pass

Calling that procedure without parts would mean parts would be bound to a NEW empty list, not the same old.

Every new pyhton programmer is bitten by this sooner or later. Most people seem to think it is stupid, and relying on it seems, to me, to go against the python mantra of "being explicit", where explicit would mean either using an instance variable in the case of OO or a closure.

thundergolfer• 5 years ago

A bit of a nit on the step-by-step rundown of what happens. At step 4 it says the Lambda “dies” but I think this is misleading. The response has completed, but the Lambda has not died. In fact the process is still running which is why the mutable state issue presents.

Even if the process died, the container stopped, and the Lambda went “cold”, I wouldn’t even call that “dead”. It’s just cold. Dead would mean totally failed or decommissioned, I think.

valinsky2• 5 years ago

Agree. Updated that part. Thanks for pointing it out.

petercooper• 5 years ago

As a Rubyist who only uses Python casually, it's neat to learn this difference! You can write very similar code in Ruby but such default objects appear to be created fresh on initialization avoiding the problem.

dragonwriter2• 5 years ago

OTOH, Ruby has a very similar footgun with mutable defaults, just with default values of Hashes, not default method arguments.

DanielVZ• 5 years ago

Linters can help a ton with stuff like this, I think pylint warns against this footgun.

Also is there a reason for not using the shorter:

var = var or []

Instead of:

var = [] if var is None else var

Asraelite• 5 years ago

I think using `or` for default fallbacks is generally considered bad practice because it disallows all falsey values, like `0` and `{}`.

It doesn't matter in this particular case because the only falsey List is `[]` anyway, but it's a bad habit.

JavaScript added the `??` operator for this specific problem.

dragonwriter2• 5 years ago

Both mypy and pyright/pylance do the right thing when things are guarded with explicit None checks and the wrong thing with “or”, so even though I prefer “or” otherwise when its gated upstrean so that None is the only falsey value it should get, I’ve taken recently to using explicit checks to make typecheckers happy.

Mehdi2277• 5 years ago

That's not been my experience. I frequently use or way and have both mypy/pyright enabled. Testing this toy function,

from typing import List, Optional

def f(x: Optional[List[int]]): x = x or [] reveal_type(x)

mypy and pyright both show List[int] as type of x at the end and correctly drop None.

patrick451• 5 years ago

I've never thought of this as a footgun. It's useful to accumulate state in a function, sort of like a static variable in c. E.g.,

  def func(x=[]):
      x.append(5)
      return x

I've always been annoyed that pylint yells at you for this.

ViViDboarder• 5 years ago

The issue is that it’s a non-apparent side effect to an outside caller.

sullyj3• 5 years ago

Having a bug caused by mutable state occur inside a "lambda function" really throws into relief what a misnomer that is.

Python Anti-Pattern

Related Stories