PEP 572: get rid of side-by-side tables; drop unnecessary TODO (#708)

Alas, the side-by-side tables caused horribly misindented code.
This commit is contained in:
Guido van Rossum 2018-07-08 15:27:52 -07:00 committed by GitHub
parent aae0d3d010
commit bd6332d245
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 107 additions and 98 deletions

View File

@ -166,16 +166,8 @@ in order to avoid ambiguities or user confusion:
foo(x = y := f(x)) # INVALID foo(x = y := f(x)) # INVALID
This rule is included to disallow excessively confusing code. This rule is included to disallow excessively confusing code, and
because parsing keyword arguments is complex enough already.
- TODO: Should we disallow using keyword arguments and top level
assignment expressions in the same call? E.g.::
# Should these be invalid?
foo(x=0, y := f(0))
bar(x := 0, y = f(x))
Regardless, ``foo(x := 0)`` should probably be valid (see below).
- Assignment expressions (even parenthesized or occurring inside other - Assignment expressions (even parenthesized or occurring inside other
constructs) are prohibited in function default values. For example, constructs) are prohibited in function default values. For example,
@ -294,7 +286,7 @@ Some examples to clarify what's technically valid or invalid::
# Valid # Valid
len(lines := f.readlines()) len(lines := f.readlines())
# Valid (TODO: Should this be disallowed?) # Valid
foo(x := 3, cat='vector') foo(x := 3, cat='vector')
# INVALID # INVALID
@ -366,121 +358,136 @@ Examples from the Python standard library
site.py site.py
^^^^^^^ ^^^^^^^
+-----------------------------------------------------+------------------------------------------------------------+
| Current code | Replaced with |
+-----------------------------------------------------+------------------------------------------------------------+
| | |
| :: | :: |
| | |
| env_base = os.environ.get("PYTHONUSERBASE", None) | if env_base := os.environ.get("PYTHONUSERBASE", None): |
| if env_base: | return env_base |
| return env_base | |
| | |
+-----------------------------------------------------+------------------------------------------------------------+
*env_base* is only used on these lines, putting its assignment on the if *env_base* is only used on these lines, putting its assignment on the if
moves it as the "header" of the block. moves it as the "header" of the block.
- Current::
env_base = os.environ.get("PYTHONUSERBASE", None)
if env_base:
return env_base
- Improved::
if env_base := os.environ.get("PYTHONUSERBASE", None):
return env_base
_pydecimal.py _pydecimal.py
^^^^^^^^^^^^^ ^^^^^^^^^^^^^
+----------------------------------------------+-----------------------------------------------------------------------+
| Current code | Replaced with |
+----------------------------------------------+-----------------------------------------------------------------------+
| | |
| :: | :: |
| | |
| if self._is_special: | if self._is_special and (ans := self._check_nans(context=context)): |
| ans = self._check_nans(context=context) | return ans |
| if ans: | |
| return ans | |
| | |
+----------------------------------------------+-----------------------------------------------------------------------+
Avoid nested if and remove one indentation level. Avoid nested if and remove one indentation level.
- Current::
if self._is_special:
ans = self._check_nans(context=context)
if ans:
return ans
- Improved::
if self._is_special and (ans := self._check_nans(context=context)):
return ans
copy.py copy.py
^^^^^^^ ^^^^^^^
Code looks more regular and avoid multiple nested if.
(See Appendix A for the origin of this example.) (See Appendix A for the origin of this example.)
+---------------------------------------------------------------+---------------------------------------------------------------+ - Current::
| Current code | Replaced with |
+---------------------------------------------------------------+---------------------------------------------------------------+
| | |
| :: | :: |
| | |
| reductor = dispatch_table.get(cls) | if reductor := dispatch_table.get(cls): |
| if reductor: | rv = reductor(x) |
| rv = reductor(x) | elif reductor := getattr(x, "__reduce_ex__", None): |
| else: | rv = reductor(4) |
| reductor = getattr(x, "__reduce_ex__", None) | elif reductor := getattr(x, "__reduce__", None): |
| if reductor: | rv = reductor() |
| rv = reductor(4) | else: |
| else: | raise Error("un(deep)copyable object of type %s" % cls) |
| reductor = getattr(x, "__reduce__", None) | |
| if reductor: | |
| rv = reductor() | |
| else: | |
| raise Error( | |
| "un(deep)copyable object of type %s" % cls) | |
+---------------------------------------------------------------+---------------------------------------------------------------+
Code looks more regular and avoid multiple nested if. reductor = dispatch_table.get(cls)
if reductor:
rv = reductor(x)
else:
reductor = getattr(x, "__reduce_ex__", None)
if reductor:
rv = reductor(4)
else:
reductor = getattr(x, "__reduce__", None)
if reductor:
rv = reductor()
else:
raise Error(
"un(deep)copyable object of type %s" % cls)
- Improved::
if reductor := dispatch_table.get(cls):
rv = reductor(x)
elif reductor := getattr(x, "__reduce_ex__", None):
rv = reductor(4)
elif reductor := getattr(x, "__reduce__", None):
rv = reductor()
else:
raise Error("un(deep)copyable object of type %s" % cls)
datetime.py datetime.py
^^^^^^^^^^^ ^^^^^^^^^^^
+-----------------------------------------------------+-----------------------------------------------------+
| Current code | Replaced with |
+-----------------------------------------------------+-----------------------------------------------------+
| | |
| :: | :: |
| | |
| s = _format_time(self._hour, self._minute, | s = _format_time(self._hour, self._minute, |
| self._second, self._microsecond, | self._second, self._microsecond, |
| timespec) | timespec) |
| tz = self._tzstr() | if tz := self._tzstr(): |
| if tz: | s += tz |
| s += tz | return s |
| return s | |
| | |
+-----------------------------------------------------+-----------------------------------------------------+
*tz* is only used for ``s += tz``, moving its assignment inside the if *tz* is only used for ``s += tz``, moving its assignment inside the if
helps to show its scope. helps to show its scope.
- Current::
s = _format_time(self._hour, self._minute,
self._second, self._microsecond,
timespec)
tz = self._tzstr()
if tz:
s += tz
return s
- Improved::
s = _format_time(self._hour, self._minute,
self._second, self._microsecond,
timespec)
if tz := self._tzstr():
s += tz
return s
sysconfig.py sysconfig.py
^^^^^^^^^^^^ ^^^^^^^^^^^^
+------------------------------------+-----------------------------------------+
| Current code | Replaced with |
+------------------------------------+-----------------------------------------+
| | |
| :: | :: |
| | |
| while True: | while line := fp.readline(): |
| line = fp.readline() | if m := define_rx.match(line): |
| if not line: | n, v = m.group(1, 2) |
| break | try: |
| m = define_rx.match(line) | v = int(v) |
| if m: | except ValueError: |
| n, v = m.group(1, 2) | pass |
| try: | vars[n] = v |
| v = int(v) | elif m := undef_rx.match(line): |
| except ValueError: | vars[m.group(1)] = 0 |
| pass | |
| vars[n] = v | |
| else: | |
| m = undef_rx.match(line) | |
| if m: | |
| vars[m.group(1)] = 0 | |
| | |
+------------------------------------+-----------------------------------------+
Calling ``fp.readline()`` in the ``while`` condition and calling Calling ``fp.readline()`` in the ``while`` condition and calling
``.match()`` on the if lines make the code more compact without making ``.match()`` on the if lines make the code more compact without making
it harder to understand. it harder to understand.
- Current::
while True:
line = fp.readline()
if not line:
break
m = define_rx.match(line)
if m:
n, v = m.group(1, 2)
try:
v = int(v)
except ValueError:
pass
vars[n] = v
else:
m = undef_rx.match(line)
if m:
vars[m.group(1)] = 0
- Improved::
while line := fp.readline():
if m := define_rx.match(line):
n, v = m.group(1, 2)
try:
v = int(v)
except ValueError:
pass
vars[n] = v
elif m := undef_rx.match(line):
vars[m.group(1)] = 0
Simplifying list comprehensions Simplifying list comprehensions
------------------------------- -------------------------------
@ -613,6 +620,8 @@ Broadly the same semantics as the current proposal, but spelled differently.
confusion or require special-casing (eg to forbid assignment within the confusion or require special-casing (eg to forbid assignment within the
headers of these statements). headers of these statements).
TODO: Explain more why this is bad, since this keeps coming up (it's also readability)
2. ``EXPR -> NAME``:: 2. ``EXPR -> NAME``::
stuff = [[f(x) -> y, x/y] for x in range(5)] stuff = [[f(x) -> y, x/y] for x in range(5)]