From 63786aae44cf7b7b1b52fb81f6e190066d53236e Mon Sep 17 00:00:00 2001 From: Tin Tvrtkovic Date: Thu, 20 May 2021 03:53:21 +0200 Subject: [PATCH 01/16] Initial implementation of a faster repr --- src/attr/_compat.py | 1 + src/attr/_make.py | 169 ++++++++++++++++++++++++++++++-------------- 2 files changed, 118 insertions(+), 52 deletions(-) diff --git a/src/attr/_compat.py b/src/attr/_compat.py index c218e2453..aa6c1f2f4 100644 --- a/src/attr/_compat.py +++ b/src/attr/_compat.py @@ -8,6 +8,7 @@ PY2 = sys.version_info[0] == 2 PYPY = platform.python_implementation() == "PyPy" +HAS_F_STRINGS = sys.version_info[:2] >= (3, 7) PY310 = sys.version_info[:2] >= (3, 10) diff --git a/src/attr/_make.py b/src/attr/_make.py index ad17109db..b0b11d610 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -12,6 +12,7 @@ from . import _config, setters from ._compat import ( + HAS_F_STRINGS, PY2, PY310, PYPY, @@ -888,7 +889,7 @@ def _create_slots_class(self): def add_repr(self, ns): self._cls_dict["__repr__"] = self._add_method_dunders( - _make_repr(self._attrs, ns=ns) + _make_repr(self._attrs, ns=ns, cls=self._cls) ) return self @@ -1873,64 +1874,128 @@ def _add_eq(cls, attrs=None): _already_repring = threading.local() +if HAS_F_STRINGS: -def _make_repr(attrs, ns): - """ - Make a repr method that includes relevant *attrs*, adding *ns* to the full - name. - """ - - # Figure out which attributes to include, and which function to use to - # format them. The a.repr value can be either bool or a custom callable. - attr_names_with_reprs = tuple( - (a.name, repr if a.repr is True else a.repr) - for a in attrs - if a.repr is not False - ) - - def __repr__(self): - """ - Automatically created by attrs. - """ - try: - working_set = _already_repring.working_set - except AttributeError: - working_set = set() - _already_repring.working_set = working_set + def _make_repr(attrs, ns, cls): + unique_filename = "repr" + # Figure out which attributes to include, and which function to use to + # format them. The a.repr value can be either bool or a custom callable. + attr_names_with_reprs = tuple( + (a.name, repr if a.repr is True else a.repr, a.init) + for a in attrs + if a.repr is not False + ) + globs = { + f"{name}_repr": r + for name, r, _ in attr_names_with_reprs + if r != repr + } + globs["_already_repring"] = _already_repring + globs["AttributeError"] = AttributeError + globs["NOTHING"] = NOTHING + attribute_fragments = [] + for name, r, i in attr_names_with_reprs: + accessor = ( + f"self.{name}" if i else f'getattr(self, "{name}", NOTHING)' + ) + fragment = ( + f"{name}={{{accessor}!r}}" + if r == repr + else f"{name}={{{name}_repr({accessor})}}" + ) + attribute_fragments.append(fragment) + repr_fragment = ", ".join(attribute_fragments) - if id(self) in working_set: - return "..." - real_cls = self.__class__ if ns is None: - qualname = getattr(real_cls, "__qualname__", None) + qualname = getattr(cls, "__qualname__", None) if qualname is not None: - class_name = qualname.rsplit(">.", 1)[-1] + cls_name_fragment = ( + '{self.__class__.__qualname__.rsplit(">.", 1)[-1]}' + ) else: - class_name = real_cls.__name__ + cls_name_fragment = "{self.__class__.__name__}" else: - class_name = ns + "." + real_cls.__name__ + cls_name_fragment = f"{ns}.{{self.__class__.__name__}}" + + lines = [] + lines.append("def __repr__(self):") + lines.append(" try:") + lines.append(" working_set = _already_repring.working_set") + lines.append(" except AttributeError:") + lines.append(" working_set = {id(self),}") + lines.append(" _already_repring.working_set = working_set") + lines.append(" else:") + lines.append(" if id(self) in working_set:") + lines.append(" return '...'") + lines.append(" else:") + lines.append(" working_set.add(id(self))") + lines.append(" try:") + lines.append(f" return f'{cls_name_fragment}({repr_fragment})'") + lines.append(" finally:") + lines.append(" working_set.remove(id(self))") + script = "\n".join(lines) + return _make_method("__repr__", script, unique_filename, globs=globs) - # Since 'self' remains on the stack (i.e.: strongly referenced) for the - # duration of this call, it's safe to depend on id(...) stability, and - # not need to track the instance and therefore worry about properties - # like weakref- or hash-ability. - working_set.add(id(self)) - try: - result = [class_name, "("] - first = True - for name, attr_repr in attr_names_with_reprs: - if first: - first = False + +else: + + def _make_repr(attrs, ns, _): + """ + Make a repr method that includes relevant *attrs*, adding *ns* to the full + name. + """ + + # Figure out which attributes to include, and which function to use to + # format them. The a.repr value can be either bool or a custom callable. + attr_names_with_reprs = tuple( + (a.name, repr if a.repr is True else a.repr) + for a in attrs + if a.repr is not False + ) + + def __repr__(self): + """ + Automatically created by attrs. + """ + try: + working_set = _already_repring.working_set + except AttributeError: + working_set = set() + _already_repring.working_set = working_set + + if id(self) in working_set: + return "..." + real_cls = self.__class__ + if ns is None: + qualname = getattr(real_cls, "__qualname__", None) + if qualname is not None: + class_name = qualname.rsplit(">.", 1)[-1] else: - result.append(", ") - result.extend( - (name, "=", attr_repr(getattr(self, name, NOTHING))) - ) - return "".join(result) + ")" - finally: - working_set.remove(id(self)) + class_name = real_cls.__name__ + else: + class_name = ns + "." + real_cls.__name__ + + # Since 'self' remains on the stack (i.e.: strongly referenced) for the + # duration of this call, it's safe to depend on id(...) stability, and + # not need to track the instance and therefore worry about properties + # like weakref- or hash-ability. + working_set.add(id(self)) + try: + result = [class_name, "("] + first = True + for name, attr_repr in attr_names_with_reprs: + if first: + first = False + else: + result.append(", ") + result.extend( + (name, "=", attr_repr(getattr(self, name, NOTHING))) + ) + return "".join(result) + ")" + finally: + working_set.remove(id(self)) - return __repr__ + return __repr__ def _add_repr(cls, ns=None, attrs=None): @@ -1940,7 +2005,7 @@ def _add_repr(cls, ns=None, attrs=None): if attrs is None: attrs = cls.__attrs_attrs__ - cls.__repr__ = _make_repr(attrs, ns) + cls.__repr__ = _make_repr(attrs, ns, cls) return cls @@ -2634,7 +2699,7 @@ def from_counting_attr(cls, name, ca, type=None): type=type, cmp=None, inherited=False, - **inst_dict + **inst_dict, ) @property From c5c58d6bb2c980c8283afd56fa3f2a470529ab08 Mon Sep 17 00:00:00 2001 From: Tin Tvrtkovic Date: Thu, 20 May 2021 03:58:58 +0200 Subject: [PATCH 02/16] Switch to positional args for _make_repr --- src/attr/_make.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index b0b11d610..e99983406 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -889,7 +889,7 @@ def _create_slots_class(self): def add_repr(self, ns): self._cls_dict["__repr__"] = self._add_method_dunders( - _make_repr(self._attrs, ns=ns, cls=self._cls) + _make_repr(self._attrs, ns, self._cls) ) return self From 8fce7bfa34b938661f3aff8008cf03dda4bbf932 Mon Sep 17 00:00:00 2001 From: Tin Tvrtkovic Date: Fri, 21 May 2021 02:22:54 +0200 Subject: [PATCH 03/16] Fix tests and changelog --- changelog.d/819.changes.rst | 1 + src/attr/_compat.py | 6 +++++- src/attr/_make.py | 18 ++++++++++++------ 3 files changed, 18 insertions(+), 7 deletions(-) create mode 100644 changelog.d/819.changes.rst diff --git a/changelog.d/819.changes.rst b/changelog.d/819.changes.rst new file mode 100644 index 000000000..826feaf14 --- /dev/null +++ b/changelog.d/819.changes.rst @@ -0,0 +1 @@ +The generated `__repr__` is significantly faster on Pythons with F-strings. diff --git a/src/attr/_compat.py b/src/attr/_compat.py index aa6c1f2f4..69329e996 100644 --- a/src/attr/_compat.py +++ b/src/attr/_compat.py @@ -8,7 +8,11 @@ PY2 = sys.version_info[0] == 2 PYPY = platform.python_implementation() == "PyPy" -HAS_F_STRINGS = sys.version_info[:2] >= (3, 7) +HAS_F_STRINGS = ( + sys.version_info[:2] >= (3, 7) + if not PYPY + else sys.version_info[:2] >= (3, 6) +) PY310 = sys.version_info[:2] >= (3, 10) diff --git a/src/attr/_make.py b/src/attr/_make.py index e99983406..131ccedb5 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1886,7 +1886,7 @@ def _make_repr(attrs, ns, cls): if a.repr is not False ) globs = { - f"{name}_repr": r + name + "_repr": r for name, r, _ in attr_names_with_reprs if r != repr } @@ -1896,12 +1896,16 @@ def _make_repr(attrs, ns, cls): attribute_fragments = [] for name, r, i in attr_names_with_reprs: accessor = ( - f"self.{name}" if i else f'getattr(self, "{name}", NOTHING)' + "self." + name + if i + else 'getattr(self, "' + name + '", NOTHING)' ) fragment = ( - f"{name}={{{accessor}!r}}" + "{name}={{{accessor}!r}}".format(name=name, accessor=accessor) if r == repr - else f"{name}={{{name}_repr({accessor})}}" + else "{name}={{{name}_repr({accessor})}}".format( + name=name, accessor=accessor + ) ) attribute_fragments.append(fragment) repr_fragment = ", ".join(attribute_fragments) @@ -1915,7 +1919,7 @@ def _make_repr(attrs, ns, cls): else: cls_name_fragment = "{self.__class__.__name__}" else: - cls_name_fragment = f"{ns}.{{self.__class__.__name__}}" + cls_name_fragment = ns + ".{self.__class__.__name__}" lines = [] lines.append("def __repr__(self):") @@ -1930,7 +1934,9 @@ def _make_repr(attrs, ns, cls): lines.append(" else:") lines.append(" working_set.add(id(self))") lines.append(" try:") - lines.append(f" return f'{cls_name_fragment}({repr_fragment})'") + lines.append( + " return f'{}({})'".format(cls_name_fragment, repr_fragment) + ) lines.append(" finally:") lines.append(" working_set.remove(id(self))") script = "\n".join(lines) From 616aba9414f8bf4ed345d7e7fe0eb85d4f3a0b10 Mon Sep 17 00:00:00 2001 From: Tin Tvrtkovic Date: Fri, 21 May 2021 12:42:24 +0200 Subject: [PATCH 04/16] Remove trailing comma for Py2 --- src/attr/_make.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index 131ccedb5..1821e0ff5 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -2705,7 +2705,7 @@ def from_counting_attr(cls, name, ca, type=None): type=type, cmp=None, inherited=False, - **inst_dict, + **inst_dict ) @property From 5261725e0ff201db6c234bb3c8c673e6edc41583 Mon Sep 17 00:00:00 2001 From: Tin Tvrtkovic Date: Sat, 22 May 2021 00:58:27 +0200 Subject: [PATCH 05/16] Fix lint --- src/attr/_make.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index 1821e0ff5..8aa1af752 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1879,7 +1879,8 @@ def _add_eq(cls, attrs=None): def _make_repr(attrs, ns, cls): unique_filename = "repr" # Figure out which attributes to include, and which function to use to - # format them. The a.repr value can be either bool or a custom callable. + # format them. The a.repr value can be either bool or a custom + # callable. attr_names_with_reprs = tuple( (a.name, repr if a.repr is True else a.repr, a.init) for a in attrs @@ -1947,12 +1948,13 @@ def _make_repr(attrs, ns, cls): def _make_repr(attrs, ns, _): """ - Make a repr method that includes relevant *attrs*, adding *ns* to the full - name. + Make a repr method that includes relevant *attrs*, adding *ns* to the + full name. """ # Figure out which attributes to include, and which function to use to - # format them. The a.repr value can be either bool or a custom callable. + # format them. The a.repr value can be either bool or a custom + # callable. attr_names_with_reprs = tuple( (a.name, repr if a.repr is True else a.repr) for a in attrs @@ -1981,10 +1983,10 @@ def __repr__(self): else: class_name = ns + "." + real_cls.__name__ - # Since 'self' remains on the stack (i.e.: strongly referenced) for the - # duration of this call, it's safe to depend on id(...) stability, and - # not need to track the instance and therefore worry about properties - # like weakref- or hash-ability. + # Since 'self' remains on the stack (i.e.: strongly referenced) + # for the duration of this call, it's safe to depend on id(...) + # stability, and not need to track the instance and therefore + # worry about properties like weakref- or hash-ability. working_set.add(id(self)) try: result = [class_name, "("] From f37fc134b66c14b03a09fc09ca59f188537e1911 Mon Sep 17 00:00:00 2001 From: Tin Tvrtkovic Date: Sat, 22 May 2021 23:55:59 +0200 Subject: [PATCH 06/16] __qualname__ is always present if f-strings work --- src/attr/_make.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index 8aa1af752..bee7c389f 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1912,13 +1912,9 @@ def _make_repr(attrs, ns, cls): repr_fragment = ", ".join(attribute_fragments) if ns is None: - qualname = getattr(cls, "__qualname__", None) - if qualname is not None: - cls_name_fragment = ( - '{self.__class__.__qualname__.rsplit(">.", 1)[-1]}' - ) - else: - cls_name_fragment = "{self.__class__.__name__}" + cls_name_fragment = ( + '{self.__class__.__qualname__.rsplit(">.", 1)[-1]}' + ) else: cls_name_fragment = ns + ".{self.__class__.__name__}" From eb091a31d2d3d54a2ae2a2bfc3a74998ec863b5d Mon Sep 17 00:00:00 2001 From: Tin Tvrtkovic Date: Sun, 23 May 2021 18:02:50 +0200 Subject: [PATCH 07/16] Fix Py2 qualname --- src/attr/_make.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index bee7c389f..d091fcaea 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1971,11 +1971,7 @@ def __repr__(self): return "..." real_cls = self.__class__ if ns is None: - qualname = getattr(real_cls, "__qualname__", None) - if qualname is not None: - class_name = qualname.rsplit(">.", 1)[-1] - else: - class_name = real_cls.__name__ + class_name = real_cls.__name__ else: class_name = ns + "." + real_cls.__name__ From 57573180350d2cddebec1abe642b966cd09ee19a Mon Sep 17 00:00:00 2001 From: Tin Tvrtkovic Date: Sun, 23 May 2021 18:09:17 +0200 Subject: [PATCH 08/16] Revert "Fix Py2 qualname" This reverts commit eb091a31d2d3d54a2ae2a2bfc3a74998ec863b5d. --- src/attr/_make.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index d091fcaea..bee7c389f 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1971,7 +1971,11 @@ def __repr__(self): return "..." real_cls = self.__class__ if ns is None: - class_name = real_cls.__name__ + qualname = getattr(real_cls, "__qualname__", None) + if qualname is not None: + class_name = qualname.rsplit(">.", 1)[-1] + else: + class_name = real_cls.__name__ else: class_name = ns + "." + real_cls.__name__ From 85fadb526d5b8a314b9abde0a4cd6a0260b94d4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tin=20Tvrtkovi=C4=87?= Date: Tue, 25 May 2021 15:16:54 +0200 Subject: [PATCH 09/16] Update src/attr/_make.py Co-authored-by: Hynek Schlawack --- src/attr/_make.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index bee7c389f..91a683582 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1972,7 +1972,11 @@ def __repr__(self): real_cls = self.__class__ if ns is None: qualname = getattr(real_cls, "__qualname__", None) - if qualname is not None: + if qualname is not None: # pragma: no cover + # This case only happens on Python 3.5 and 3.6. We exclude + # it from coverage, because we don't want to slow down our + # test suite by running them under coverage too for this + # one line. class_name = qualname.rsplit(">.", 1)[-1] else: class_name = real_cls.__name__ From 08ef044d936967a45adfb1fc786b8624267e6b8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tin=20Tvrtkovi=C4=87?= Date: Tue, 25 May 2021 15:17:02 +0200 Subject: [PATCH 10/16] Update changelog.d/819.changes.rst Co-authored-by: Hynek Schlawack --- changelog.d/819.changes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/819.changes.rst b/changelog.d/819.changes.rst index 826feaf14..eb45d6168 100644 --- a/changelog.d/819.changes.rst +++ b/changelog.d/819.changes.rst @@ -1 +1 @@ -The generated `__repr__` is significantly faster on Pythons with F-strings. +The generated ``__repr__`` is significantly faster on Pythons with F-strings. From 311f743cde0737fb3aaaadcc5121516195b404c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tin=20Tvrtkovi=C4=87?= Date: Wed, 26 May 2021 14:39:15 +0200 Subject: [PATCH 11/16] Update src/attr/_make.py Co-authored-by: Hynek Schlawack --- src/attr/_make.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index 91a683582..692f1cc2f 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1882,7 +1882,7 @@ def _make_repr(attrs, ns, cls): # format them. The a.repr value can be either bool or a custom # callable. attr_names_with_reprs = tuple( - (a.name, repr if a.repr is True else a.repr, a.init) + (a.name, (repr if a.repr is True else a.repr), a.init) for a in attrs if a.repr is not False ) From 31ef67fcc13959cba1e6aebb6efbbd69584ba97d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tin=20Tvrtkovi=C4=87?= Date: Wed, 26 May 2021 14:39:26 +0200 Subject: [PATCH 12/16] Update src/attr/_make.py Co-authored-by: Hynek Schlawack --- src/attr/_make.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index 692f1cc2f..e05166db3 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1902,7 +1902,7 @@ def _make_repr(attrs, ns, cls): else 'getattr(self, "' + name + '", NOTHING)' ) fragment = ( - "{name}={{{accessor}!r}}".format(name=name, accessor=accessor) + "%s={%s!r}" % (name, accessor) if r == repr else "{name}={{{name}_repr({accessor})}}".format( name=name, accessor=accessor From 313b885c02e90ac9929a7881c7127edd4b0f6281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tin=20Tvrtkovi=C4=87?= Date: Wed, 26 May 2021 14:39:37 +0200 Subject: [PATCH 13/16] Update src/attr/_make.py Co-authored-by: Hynek Schlawack --- src/attr/_make.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index e05166db3..0b185cf26 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1904,8 +1904,7 @@ def _make_repr(attrs, ns, cls): fragment = ( "%s={%s!r}" % (name, accessor) if r == repr - else "{name}={{{name}_repr({accessor})}}".format( - name=name, accessor=accessor + else "%s={%s_repr(%s)}" % (name, name, accessor) ) ) attribute_fragments.append(fragment) From 2c6f584bbb5d4c13af9bcb0b6369776283cf498d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tin=20Tvrtkovi=C4=87?= Date: Wed, 26 May 2021 14:39:45 +0200 Subject: [PATCH 14/16] Update src/attr/_make.py Co-authored-by: Hynek Schlawack --- src/attr/_make.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index 0b185cf26..e54b8d72a 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1931,7 +1931,7 @@ def _make_repr(attrs, ns, cls): lines.append(" working_set.add(id(self))") lines.append(" try:") lines.append( - " return f'{}({})'".format(cls_name_fragment, repr_fragment) + " return f'%s(%s)'" % (cls_name_fragment, repr_fragment) ) lines.append(" finally:") lines.append(" working_set.remove(id(self))") From efaa52eb88605885b5c747a5139ef4aace78ad7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tin=20Tvrtkovi=C4=87?= Date: Wed, 26 May 2021 14:40:25 +0200 Subject: [PATCH 15/16] Update src/attr/_make.py Co-authored-by: Hynek Schlawack --- src/attr/_make.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index e54b8d72a..301ddefcc 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1935,8 +1935,10 @@ def _make_repr(attrs, ns, cls): ) lines.append(" finally:") lines.append(" working_set.remove(id(self))") - script = "\n".join(lines) - return _make_method("__repr__", script, unique_filename, globs=globs) + + return _make_method( + "__repr__", "\n".join(lines), unique_filename, globs=globs + ) else: From b4187af1c1956247671a06504036c8050b34a9ca Mon Sep 17 00:00:00 2001 From: Tin Tvrtkovic Date: Wed, 26 May 2021 18:09:12 +0200 Subject: [PATCH 16/16] Fix syntax --- src/attr/_make.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/attr/_make.py b/src/attr/_make.py index 301ddefcc..658eb047b 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -1905,7 +1905,6 @@ def _make_repr(attrs, ns, cls): "%s={%s!r}" % (name, accessor) if r == repr else "%s={%s_repr(%s)}" % (name, name, accessor) - ) ) attribute_fragments.append(fragment) repr_fragment = ", ".join(attribute_fragments)