bslatkin / effectivepython

Effective Python: Second Edition — Source Code and Errata for the Book
https://effectivepython.com
2.2k stars 710 forks source link

Item 29: Guidance on variable leakage is no longer true #83

Open bslatkin opened 4 years ago

bslatkin commented 4 years ago

Reported by Mr. Kurokawa.

This sentence and the preceding example:

It’s better not to leak loop variables, so I recommend using assignment expressions only in the condition part of a comprehension.

are no longer true. Python was changed between when this item was written and now. This is noted in the PEP:

https://www.python.org/dev/peps/pep-0572/#why-not-use-a-sublocal-scope-and-prevent-namespace-pollution

Why not use a sublocal scope and prevent namespace pollution? Previous revisions of this proposal involved sublocal scope (restricted to a single statement), preventing name leakage and namespace pollution. While a definite advantage in a number of situations, this increases complexity in many others, and the costs are not justified by the benefits. In the interests of language simplicity, the name bindings created here are exactly equivalent to any other name bindings, including that usage at class or module scope will create externally-visible names. This is no different from for loops or other constructs, and can be solved the same way: del the name once it is no longer needed, or prefix it with an underscore.

Example code that shows how the example in the book is no longer true (because tenth leaks):

>>> name
Traceback (most recent call last):
  File "<pyshell#0>", line 1, in <module>
    name
NameError: name 'name' is not defined
>>> stock = {
    'nails': 125,
    'screws': 35,
    'wingnuts': 8,
    'washers': 24,
}
>>> order = ['screws', 'wingnuts', 'clips']
>>> count
Traceback (most recent call last):
  File "<pyshell#3>", line 1, in <module>
    count
NameError: name 'count' is not defined
>>> result = {name: tenth for name, count in stock.items()
          if (tenth := count // 10) > 0}
>>> result
{'nails': 12, 'screws': 3, 'washers': 2}
>>> name
Traceback (most recent call last):
  File "<pyshell#6>", line 1, in <module>
    name
NameError: name 'name' is not defined
>>> count
Traceback (most recent call last):
  File "<pyshell#7>", line 1, in <module>
    count
NameError: name 'count' is not defined
>>> tenth
2
bslatkin commented 4 years ago

Here's the diff for the main part of the item to fix this:

-If a comprehension uses the walrus operator in the value part of the comprehension and doesn’t have a condition, it’ll leak the loop variable into the containing scope (see Item 21: “Know How Closures Interact with Variable Scope” for background):
+When a comprehension uses walrus operators, any corresponding variable names will be leaked into the containing scope (see Item 21: “Know How Closures Interact with Variable Scope” for background):

 ```python
-half = [(last := count // 2) for count in stock.values()]
-print(f'Last item of {half} is {last}')
+half = [(squared := last ** 2)
+        for count in stock.values()
+        if (last := count // 2) > 10]
+print(f'Last item of {half} is {last} ** 2 = {squared}')
-Last item of [62, 17, 4, 12] is 12
+Last item of [3844, 289, 144] is 12 ** 2 = 144

-This leakage of the loop variable is similar to what happens with a normal for loop: +The leakage of these variable names is similar to what happens with a normal for loop:

-for count in stock.values():  # Leaks loop variable
-    pass
-print(f'Last item of {list(stock.values())} is {count}')
+for count in stock.values():
+    last = count // 2
+    squared = last ** 2
+
+print(f'{count} // 2 = {last}; {last} ** 2 = {squared}')
-Last item of [125, 35, 8, 24] is 24
+24 // 2 = 12; 12 ** 2 = 144

-However, similar leakage doesn’t happen for the loop variables from comprehensions: +However, this leakage behavior can be surprising because when comprehensions don’t use assignment expressions the loop variable names won’t leak:


And this part needs to be removed entirely:

-It’s better not to leak loop variables, so I recommend using assignment expressions only in the condition part of a comprehension.

RhysU commented 3 years ago

Thank you for reporting this errata. The original content felt odd from a language perspective and some experimentation in Python 3.8.1 showed a confusing result.