Trivial dataclass cleanups: (GH-6218)

miss-islington · ericvsmith · web-flow · commit 02c19a6fc08d · 2018-03-24T14:42:28.000-07:00
- When adding a single element to a list, use .append() instead of += and creating a new list. - For consistency, import the copy module, instead of just deepcopy. This leaves only a module at the class level, instead of a function. - Improve some comments. - Improve some whitespace. - Use tuples instead of lists. - Simplify a test. (cherry picked from commit f96ddad) Co-authored-by: Eric V. Smith <ericvsmith@users.noreply.github.com>
diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py
@@ -1,6 +1,6 @@
 import sys
+import copy
 import types
-from copy import deepcopy
 import inspect
 
 __all__ = ['dataclass',
@@ -137,7 +137,7 @@
 # For boxes that are blank, __hash__ is untouched and therefore
 #  inherited from the base class.  If the base is object, then
 #  id-based hashing is used.
-# Note that a class may have already __hash__=None if it specified an
+# Note that a class may already have __hash__=None if it specified an
 #  __eq__ method in the class body (not one that was created by
 #  @dataclass).
 # See _hash_action (below) for a coded version of this table.
@@ -147,7 +147,7 @@
 class FrozenInstanceError(AttributeError): pass
 
 # A sentinel object for default values to signal that a
-#  default-factory will be used.
+#  default factory will be used.
 # This is given a nice repr() which will appear in the function
 #  signature of dataclasses' constructors.
 class _HAS_DEFAULT_FACTORY_CLASS:
@@ -249,6 +249,7 @@ class _DataclassParams:
                  'unsafe_hash',
                  'frozen',
                  )
+
     def __init__(self, init, repr, eq, order, unsafe_hash, frozen):
         self.init = init
         self.repr = repr
@@ -267,6 +268,7 @@ def __repr__(self):
                 f'frozen={self.frozen}'
                 ')')
 
+
 # This function is used instead of exposing Field creation directly,
 #  so that a type checker can be told (via overloads) that this is a
 #  function whose type depends on its parameters.
@@ -307,6 +309,8 @@ def _tuple_str(obj_name, fields):
 def _create_fn(name, args, body, *, globals=None, locals=None,
                return_type=MISSING):
     # Note that we mutate locals when exec() is called. Caller beware!
+    #  The only callers are internal to this module, so no worries
+    #  about external callers.
     if locals is None:
         locals = {}
     return_annotation = ''
@@ -429,26 +433,25 @@ def _init_fn(fields, frozen, has_post_init, self_name):
 
     body_lines = []
     for f in fields:
-        # Do not initialize the pseudo-fields, only the real ones.
         line = _field_init(f, frozen, globals, self_name)
-        if line is not None:
-            # line is None means that this field doesn't require
-            #  initialization. Just skip it.
+        # line is None means that this field doesn't require
+        #  initialization (it's a pseudo-field).  Just skip it.
+        if line:
             body_lines.append(line)
 
     # Does this class have a post-init function?
     if has_post_init:
         params_str = ','.join(f.name for f in fields
                               if f._field_type is _FIELD_INITVAR)
-        body_lines += [f'{self_name}.{_POST_INIT_NAME}({params_str})']
+        body_lines.append(f'{self_name}.{_POST_INIT_NAME}({params_str})')
 
     # If no body lines, use 'pass'.
     if not body_lines:
         body_lines = ['pass']
 
     locals = {f'_type_{f.name}': f.type for f in fields}
     return _create_fn('__init__',
-                      [self_name] +[_init_param(f) for f in fields if f.init],
+                      [self_name] + [_init_param(f) for f in fields if f.init],
                       body_lines,
                       locals=locals,
                       globals=globals,
@@ -457,7 +460,7 @@ def _init_fn(fields, frozen, has_post_init, self_name):
 
 def _repr_fn(fields):
     return _create_fn('__repr__',
-                      ['self'],
+                      ('self',),
                       ['return self.__class__.__qualname__ + f"(' +
                        ', '.join([f"{f.name}={{self.{f.name}!r}}"
                                   for f in fields]) +
@@ -496,7 +499,7 @@ def _cmp_fn(name, op, self_tuple, other_tuple):
     #  '(other.x,other.y)'.
 
     return _create_fn(name,
-                      ['self', 'other'],
+                      ('self', 'other'),
                       [ 'if other.__class__ is self.__class__:',
                        f' return {self_tuple}{op}{other_tuple}',
                         'return NotImplemented'])
@@ -505,12 +508,12 @@ def _cmp_fn(name, op, self_tuple, other_tuple):
 def _hash_fn(fields):
     self_tuple = _tuple_str('self', fields)
     return _create_fn('__hash__',
-                      ['self'],
+                      ('self',),
                       [f'return hash({self_tuple})'])
 
 
 def _get_field(cls, a_name, a_type):
-    # Return a Field object, for this field name and type.  ClassVars
+    # Return a Field object for this field name and type.  ClassVars
     #  and InitVars are also returned, but marked as such (see
     #  f._field_type).
 
@@ -560,9 +563,9 @@ def _get_field(cls, a_name, a_type):
             raise TypeError(f'field {f.name} cannot have a '
                             'default factory')
         # Should I check for other field settings? default_factory
-        #  seems the most serious to check for. Maybe add others.  For
-        #  example, how about init=False (or really,
-        #  init=<not-the-default-init-value>)? It makes no sense for
+        #  seems the most serious to check for.  Maybe add others.
+        #  For example, how about init=False (or really,
+        #  init=<not-the-default-init-value>)?  It makes no sense for
         #  ClassVar and InitVar to specify init=<anything>.
 
     # For real fields, disallow mutable defaults for known types.
@@ -903,7 +906,7 @@ def _asdict_inner(obj, dict_factory):
         return type(obj)((_asdict_inner(k, dict_factory), _asdict_inner(v, dict_factory))
                           for k, v in obj.items())
     else:
-        return deepcopy(obj)
+        return copy.deepcopy(obj)
 
 
 def astuple(obj, *, tuple_factory=tuple):
@@ -943,7 +946,7 @@ def _astuple_inner(obj, tuple_factory):
         return type(obj)((_astuple_inner(k, tuple_factory), _astuple_inner(v, tuple_factory))
                           for k, v in obj.items())
     else:
-        return deepcopy(obj)
+        return copy.deepcopy(obj)
 
 
 def make_dataclass(cls_name, fields, *, bases=(), namespace=None, init=True,
@@ -1032,9 +1035,9 @@ class C:
         if f.name not in changes:
             changes[f.name] = getattr(obj, f.name)
 
-    # Create the new object, which calls __init__() and __post_init__
-    #  (if defined), using all of the init fields we've added and/or
-    #  left in 'changes'.
-    # If there are values supplied in changes that aren't fields, this
-    #  will correctly raise a TypeError.
+    # Create the new object, which calls __init__() and
+    #  __post_init__() (if defined), using all of the init fields
+    #  we've added and/or left in 'changes'.  If there are values
+    #  supplied in changes that aren't fields, this will correctly
+    #  raise a TypeError.
     return obj.__class__(**changes)
