From 657e1e345ab3c028227df3583952cb6ec74bda33 Mon Sep 17 00:00:00 2001 From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com> Date: Sun, 11 Sep 2022 14:03:55 +0200 Subject: [PATCH 1/5] Readable string representations for TelegramObject --- telegram/_telegramobject.py | 23 +++++++++++++++++++---- tests/test_telegramobject.py | 32 +++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index a4c9e2f3578..b87fd28ff2e 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -44,8 +44,12 @@ class TelegramObject: assert telegram_object.get_bot() is copy.deepcopy(telegram_object).get_bot() .. versionchanged:: 20.0 - ``telegram_object['from']`` will look up the key ``from_user``. This is to account for - special cases like :attr:`Message.from_user` that deviate from the official Bot API. + + * ``telegram_object['from']`` will look up the key ``from_user``. This is to account for + special cases like :attr:`Message.from_user` that deviate from the official Bot API. + * ``str(telegram_object)`` and ``repr(telegram_object)`` now both return a string of the + form ``ClassName(attr_1=value_1, attr_2=value_2, ...)``, where attributes with the + value :obj:`None` are omitted. """ # type hints in __new__ are not read by mypy (https://github.com/python/mypy/issues/1021). As a @@ -67,8 +71,19 @@ def __new__(cls, *args: object, **kwargs: object) -> "TelegramObject": instance._bot = None return instance - def __str__(self) -> str: - return str(self.to_dict()) + def __repr__(self) -> str: + """ + * `__repr__` goal is to be unambiguous + * `__str__` goal is to be readable + * `str()` calls `__repr__`, if `__str__` is not defined + + In our case "unambiguous" and "readable" largely coincide, so we can use the same logic. + + Prints the object in the form `ClassName(arg=value, arg=value, ...)` + """ + as_dict = self._get_attrs(recursive=False, include_private=False) + contents = {", ".join(f"{k}={v!r}" for k, v in as_dict.items() if v is not None)} + return f"{self.__class__.__name__}({contents})" def __getitem__(self, item: str) -> object: if item == "from": diff --git a/tests/test_telegramobject.py b/tests/test_telegramobject.py index c721149a90a..d0cc4e2a66e 100644 --- a/tests/test_telegramobject.py +++ b/tests/test_telegramobject.py @@ -22,7 +22,7 @@ import pytest -from telegram import Chat, Message, PhotoSize, TelegramObject, User +from telegram import BotCommand, Chat, Message, PhotoSize, TelegramObject, User class TestTelegramObject: @@ -167,3 +167,33 @@ def test_deepcopy_subclass_telegram_obj(self, bot): assert d._private == s._private # Can't test for identity since two equal strings is True assert d._bot == s._bot and d._bot is s._bot assert d.normal == s.normal + + def test_string_representation(self): + class TGO(TelegramObject): + def __init__(self): + super().__init__() + self.string_attr = "string" + self.int_attr = 42 + self.to_attr = BotCommand("command", "description") + self.list_attr = [ + BotCommand("command_1", "description_1"), + BotCommand("command_2", "description_2"), + ] + self.dict_attr = { + BotCommand("command_1", "description_1"): BotCommand( + "command_2", "description_2" + ) + } + # Should not be included in string representation + self.none_attr = None + + expected = ( + "TGO(string_attr='string', int_attr=42, to_attr=BotCommand(" + "description='description', command='command'), list_attr=[BotCommand(" + "description='description_1', command='command_1'), BotCommand(" + "description='description_2', command='command_2')], dict_attr={BotCommand(" + "description='description_1', command='command_1'): BotCommand(" + "description='description_2', command='command_2')})" + ) + assert str(TGO()) == expected + assert repr(TGO()) == expected From 63574870c7fab80075464b35c712d52526a7625e Mon Sep 17 00:00:00 2001 From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com> Date: Sun, 11 Sep 2022 21:08:33 +0200 Subject: [PATCH 2/5] Fix rookie mistake --- telegram/_telegramobject.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index b87fd28ff2e..5f7fcfa12a5 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -82,7 +82,7 @@ def __repr__(self) -> str: Prints the object in the form `ClassName(arg=value, arg=value, ...)` """ as_dict = self._get_attrs(recursive=False, include_private=False) - contents = {", ".join(f"{k}={v!r}" for k, v in as_dict.items() if v is not None)} + contents = ", ".join(f"{k}={v!r}" for k, v in as_dict.items() if v is not None) return f"{self.__class__.__name__}({contents})" def __getitem__(self, item: str) -> object: From d3b650217972d63e3d34b91b81098d13c51394b8 Mon Sep 17 00:00:00 2001 From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com> Date: Thu, 20 Oct 2022 17:58:01 +0200 Subject: [PATCH 3/5] Handle api_kwargs and list attributes alphabetically --- telegram/_telegramobject.py | 9 ++++++++- tests/test_telegramobject.py | 34 +++++++++++++++++++++++----------- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index 8e1e27d2e7c..61e2584a8fc 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -114,7 +114,14 @@ def __repr__(self) -> str: Prints the object in the form `ClassName(arg=value, arg=value, ...)` """ as_dict = self._get_attrs(recursive=False, include_private=False) - contents = ", ".join(f"{k}={v!r}" for k, v in as_dict.items() if v is not None) + + if not self.api_kwargs: + # Drop api_kwargs from the representation, if empty + as_dict.pop("api_kwargs", None) + + contents = ", ".join( + f"{k}={as_dict[k]!r}" for k in sorted(as_dict.keys()) if as_dict[k] is not None + ) return f"{self.__class__.__name__}({contents})" def __getitem__(self, item: str) -> object: diff --git a/tests/test_telegramobject.py b/tests/test_telegramobject.py index 7a56ed295fe..8c6e5599f90 100644 --- a/tests/test_telegramobject.py +++ b/tests/test_telegramobject.py @@ -287,8 +287,8 @@ def test_deepcopy_subclass_telegram_obj(self, bot): def test_string_representation(self): class TGO(TelegramObject): - def __init__(self): - super().__init__() + def __init__(self, api_kwargs=None): + super().__init__(api_kwargs=api_kwargs) self.string_attr = "string" self.int_attr = 42 self.to_attr = BotCommand("command", "description") @@ -304,13 +304,25 @@ def __init__(self): # Should not be included in string representation self.none_attr = None - expected = ( - "TGO(string_attr='string', int_attr=42, to_attr=BotCommand(" - "description='description', command='command'), list_attr=[BotCommand(" - "description='description_1', command='command_1'), BotCommand(" - "description='description_2', command='command_2')], dict_attr={BotCommand(" - "description='description_1', command='command_1'): BotCommand(" - "description='description_2', command='command_2')})" + expected_without_api_kwargs = ( + "TGO(dict_attr={BotCommand(command='command_1', description='description_1'): " + "BotCommand(command='command_2', description='description_2')}, int_attr=42, " + "list_attr=[BotCommand(command='command_1', description='description_1'), " + "BotCommand(command='command_2', description='description_2')], " + "string_attr='string', to_attr=BotCommand(command='command', " + "description='description'))" + ) + assert str(TGO()) == expected_without_api_kwargs + assert repr(TGO()) == expected_without_api_kwargs + + expected_with_api_kwargs = ( + "TGO(api_kwargs={'foo': 'bar'}, dict_attr={BotCommand(command='command_1', " + "description='description_1'): BotCommand(command='command_2', " + "description='description_2')}, int_attr=42, " + "list_attr=[BotCommand(command='command_1', description='description_1'), " + "BotCommand(command='command_2', description='description_2')], " + "string_attr='string', to_attr=BotCommand(command='command', " + "description='description'))" ) - assert str(TGO()) == expected - assert repr(TGO()) == expected + assert str(TGO(api_kwargs={"foo": "bar"})) == expected_with_api_kwargs + assert str(TGO(api_kwargs={"foo": "bar"})) == expected_with_api_kwargs From d3c49d2ed958037e939fa4631af777492ce56d87 Mon Sep 17 00:00:00 2001 From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com> Date: Thu, 20 Oct 2022 18:14:27 +0200 Subject: [PATCH 4/5] fix docs --- telegram/_telegramobject.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index 61e2584a8fc..f4e2f6d0cc9 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -53,8 +53,8 @@ class TelegramObject: :meth:`set_bot` and :meth:`get_bot` instead. * Removed the possibility to pass arbitrary keyword arguments for several subclasses. * ``str(telegram_object)`` and ``repr(telegram_object)`` now both return a string of the - form ``ClassName(attr_1=value_1, attr_2=value_2, ...)``, where attributes with the - value :obj:`None` are omitted. + form ``ClassName(attr_1=value_1, attr_2=value_2, ...)``, where attributes with the + value :obj:`None` are omitted. Arguments: api_kwargs (Dict[:obj:`str`, any], optional): |toapikwargsarg| From 4d0ab5d425578e8d982937eb6544099d169c4c46 Mon Sep 17 00:00:00 2001 From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com> Date: Sat, 29 Oct 2022 15:47:11 +0200 Subject: [PATCH 5/5] review & further adjustments --- docs/source/telegram.telegramobject.rst | 1 + telegram/_telegramobject.py | 36 +++++++++++++++++-------- tests/test_telegramobject.py | 4 ++- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/docs/source/telegram.telegramobject.rst b/docs/source/telegram.telegramobject.rst index 61432be1838..9a3d85d6c97 100644 --- a/docs/source/telegram.telegramobject.rst +++ b/docs/source/telegram.telegramobject.rst @@ -4,3 +4,4 @@ telegram.TelegramObject .. autoclass:: telegram.TelegramObject :members: :show-inheritance: + :special-members: __repr__ diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index f4e2f6d0cc9..a9e5834925a 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -21,7 +21,7 @@ import json from copy import deepcopy from itertools import chain -from typing import TYPE_CHECKING, Dict, List, Optional, Set, Tuple, Type, TypeVar, Union +from typing import TYPE_CHECKING, Dict, List, Optional, Set, Sized, Tuple, Type, TypeVar, Union from telegram._utils.types import JSONDict from telegram._utils.warnings import warn @@ -52,9 +52,9 @@ class TelegramObject: * Removed argument and attribute ``bot`` for several subclasses. Use :meth:`set_bot` and :meth:`get_bot` instead. * Removed the possibility to pass arbitrary keyword arguments for several subclasses. - * ``str(telegram_object)`` and ``repr(telegram_object)`` now both return a string of the - form ``ClassName(attr_1=value_1, attr_2=value_2, ...)``, where attributes with the - value :obj:`None` are omitted. + * String representations objects of this type was overhauled. See :meth:`__repr__` for + details. As this class doesn't implement :meth:`object.__str__`, the default + implementation will be used, which is equivalent to :meth:`__repr__`. Arguments: api_kwargs (Dict[:obj:`str`, any], optional): |toapikwargsarg| @@ -104,15 +104,21 @@ def _apply_api_kwargs(self) -> None: setattr(self, key, self.api_kwargs.pop(key)) def __repr__(self) -> str: - """ - * `__repr__` goal is to be unambiguous - * `__str__` goal is to be readable - * `str()` calls `__repr__`, if `__str__` is not defined + """Gives a string representation of this object in the form + ``ClassName(attr_1=value_1, attr_2=value_2, ...)``, where attributes are omitted if they + have the value :obj:`None` or empty instances of :class:`collections.abc.Sized` (e.g. + :class:`list`, :class:`dict`, :class:`set`, :class:`str`, etc.). - In our case "unambiguous" and "readable" largely coincide, so we can use the same logic. + As this class doesn't implement :meth:`object.__str__`, the default implementation + will be used, which is equivalent to :meth:`__repr__`. - Prints the object in the form `ClassName(arg=value, arg=value, ...)` + Returns: + :obj:`str` """ + # * `__repr__` goal is to be unambiguous + # * `__str__` goal is to be readable + # * `str()` calls `__repr__`, if `__str__` is not defined + # In our case "unambiguous" and "readable" largely coincide, so we can use the same logic. as_dict = self._get_attrs(recursive=False, include_private=False) if not self.api_kwargs: @@ -120,7 +126,15 @@ def __repr__(self) -> str: as_dict.pop("api_kwargs", None) contents = ", ".join( - f"{k}={as_dict[k]!r}" for k in sorted(as_dict.keys()) if as_dict[k] is not None + f"{k}={as_dict[k]!r}" + for k in sorted(as_dict.keys()) + if ( + as_dict[k] is not None + and not ( + isinstance(as_dict[k], Sized) + and len(as_dict[k]) == 0 # type: ignore[arg-type] + ) + ) ) return f"{self.__class__.__name__}({contents})" diff --git a/tests/test_telegramobject.py b/tests/test_telegramobject.py index 8c6e5599f90..e636a0068c1 100644 --- a/tests/test_telegramobject.py +++ b/tests/test_telegramobject.py @@ -301,6 +301,8 @@ def __init__(self, api_kwargs=None): "command_2", "description_2" ) } + self.empty_tuple_attrs = () + self.empty_str_attribute = "" # Should not be included in string representation self.none_attr = None @@ -325,4 +327,4 @@ def __init__(self, api_kwargs=None): "description='description'))" ) assert str(TGO(api_kwargs={"foo": "bar"})) == expected_with_api_kwargs - assert str(TGO(api_kwargs={"foo": "bar"})) == expected_with_api_kwargs + assert repr(TGO(api_kwargs={"foo": "bar"})) == expected_with_api_kwargs