From 32e225bd10467901eccb6d418b739414853f6b23 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Sun, 2 Oct 2022 01:16:05 +0530 Subject: [PATCH 01/11] add recursion parameter to to_dict And fix _get_attrs for non-slotted classes --- telegram/_telegramobject.py | 27 ++++++++++++++++++--------- tests/test_telegramobject.py | 20 ++++++++++++++++++++ 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index 34e9ba08ad9..5b4cbe971bc 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -169,17 +169,19 @@ def _get_attrs( """ data = {} - if not recursive: - try: - # __dict__ has attrs from superclasses, so no need to put in the for loop below - data.update(self.__dict__) - except AttributeError: - pass + try: + # __dict__ has attrs from superclasses, so no need to recompute/duplicate + data.update(self.__dict__) + except AttributeError: + pass + + tuple_data = tuple(data) # We want to get all attributes for the class, using self.__slots__ only includes the # attributes used by that class itself, and not its superclass(es). Hence, we get its MRO # and then get their attributes. The `[:-1]` slice excludes the `object` class for cls in self.__class__.__mro__[:-1]: - for key in cls.__slots__: # type: ignore[attr-defined] + # add the class's slots with the user defined subclass __dict__ (class has no slots) + for key in tuple(cls.__slots__) + tuple_data: # type: ignore[attr-defined] if not include_private and key.startswith("_"): continue @@ -279,16 +281,23 @@ def to_json(self) -> str: """ return json.dumps(self.to_dict()) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """Gives representation of object as :obj:`dict`. .. versionchanged:: 20.0 Now includes all entries of :attr:`api_kwargs`. + Args: + recursive (:obj:`bool`, optional): If :obj:`True`, will convert any TelegramObjects + (if found) in the attributes to a dictionary. Else, preserves it as an object + itself. Defaults to :obj:`True`. + + .. versionadded:: 20.0 + Returns: :obj:`dict` """ - out = self._get_attrs(recursive=True) + out = self._get_attrs(recursive=recursive) out.update(out.pop("api_kwargs", {})) # type: ignore[call-overload] return out diff --git a/tests/test_telegramobject.py b/tests/test_telegramobject.py index 35596be1146..7c2184f7877 100644 --- a/tests/test_telegramobject.py +++ b/tests/test_telegramobject.py @@ -138,6 +138,26 @@ def test_to_dict_api_kwargs(self): to = TelegramObject(api_kwargs={"foo": "bar"}) assert to.to_dict() == {"foo": "bar"} + def test_to_dict_recursion(self): + class Recusive(TelegramObject): + def __init__(self): + super().__init__() + self.recursive = "recursive" + + class SubClass(TelegramObject): + def __init__(self): + super().__init__() + self.subclass = Recusive() + + to = SubClass() + to_dict_no_recurse = to.to_dict(recursive=False) + assert to_dict_no_recurse + assert isinstance(to_dict_no_recurse["subclass"], Recusive) + to_dict_recuse = to.to_dict(recursive=True) + assert to_dict_recuse + assert isinstance(to_dict_recuse["subclass"], dict) + assert to_dict_recuse["subclass"]["recursive"] == "recursive" + def test_slot_behaviour(self, mro_slots): inst = TelegramObject() for attr in inst.__slots__: From 7939e7e529f8f555164ace71522dd67957e99e11 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Sun, 2 Oct 2022 01:39:51 +0530 Subject: [PATCH 02/11] change subclass signature --- telegram/_bot.py | 2 +- telegram/_chatinvitelink.py | 4 ++-- telegram/_chatjoinrequest.py | 4 ++-- telegram/_chatmember.py | 4 ++-- telegram/_chatmemberupdated.py | 4 ++-- telegram/_files/inputmedia.py | 4 ++-- telegram/_files/sticker.py | 4 ++-- telegram/_games/game.py | 4 ++-- telegram/_inline/inlinekeyboardmarkup.py | 4 ++-- telegram/_inline/inlinequeryresult.py | 4 ++-- telegram/_inline/inputinvoicemessagecontent.py | 4 ++-- telegram/_inline/inputtextmessagecontent.py | 4 ++-- telegram/_menubutton.py | 4 ++-- telegram/_message.py | 4 ++-- telegram/_passport/credentials.py | 12 ++++++------ telegram/_passport/encryptedpassportelement.py | 4 ++-- telegram/_passport/passportdata.py | 4 ++-- telegram/_payment/shippingoption.py | 4 ++-- telegram/_poll.py | 4 ++-- telegram/_replykeyboardmarkup.py | 4 ++-- telegram/_userprofilephotos.py | 4 ++-- telegram/_videochat.py | 8 ++++---- 22 files changed, 49 insertions(+), 49 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index cb4e00c6b77..df0771a15d5 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -8205,7 +8205,7 @@ async def create_invoice_link( api_kwargs=api_kwargs, ) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" data: JSONDict = {"id": self.id, "username": self.username, "first_name": self.first_name} diff --git a/telegram/_chatinvitelink.py b/telegram/_chatinvitelink.py index 3701b1d0a8c..f0ff6543ce8 100644 --- a/telegram/_chatinvitelink.py +++ b/telegram/_chatinvitelink.py @@ -151,9 +151,9 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["ChatInviteLi return super().de_json(data=data, bot=bot) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["expire_date"] = to_timestamp(self.expire_date) diff --git a/telegram/_chatjoinrequest.py b/telegram/_chatjoinrequest.py index 4edb03a906e..2869df129e5 100644 --- a/telegram/_chatjoinrequest.py +++ b/telegram/_chatjoinrequest.py @@ -103,9 +103,9 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["ChatJoinRequ return super().de_json(data=data, bot=bot) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["date"] = to_timestamp(self.date) diff --git a/telegram/_chatmember.py b/telegram/_chatmember.py index 7d8f24ced55..01b40f7e195 100644 --- a/telegram/_chatmember.py +++ b/telegram/_chatmember.py @@ -123,9 +123,9 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["ChatMember"] return super().de_json(data=data, bot=bot) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) if data.get("until_date", False): data["until_date"] = to_timestamp(data["until_date"]) diff --git a/telegram/_chatmemberupdated.py b/telegram/_chatmemberupdated.py index cceb469c78f..e0abc5f03c9 100644 --- a/telegram/_chatmemberupdated.py +++ b/telegram/_chatmemberupdated.py @@ -122,9 +122,9 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["ChatMemberUp return super().de_json(data=data, bot=bot) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) # Required data["date"] = to_timestamp(self.date) diff --git a/telegram/_files/inputmedia.py b/telegram/_files/inputmedia.py index 1452135b650..3174aca5813 100644 --- a/telegram/_files/inputmedia.py +++ b/telegram/_files/inputmedia.py @@ -90,9 +90,9 @@ def __init__( self.caption_entities = caption_entities self.parse_mode = parse_mode - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) if self.caption_entities: data["caption_entities"] = [ce.to_dict() for ce in self.caption_entities] diff --git a/telegram/_files/sticker.py b/telegram/_files/sticker.py index dfd23b6ab26..64e42c34020 100644 --- a/telegram/_files/sticker.py +++ b/telegram/_files/sticker.py @@ -282,9 +282,9 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["StickerSet"] return super()._de_json(data=data, bot=bot, api_kwargs=api_kwargs) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["stickers"] = [s.to_dict() for s in data.get("stickers")] # type: ignore[union-attr] diff --git a/telegram/_games/game.py b/telegram/_games/game.py index 6fc191fda1b..91d31b17e09 100644 --- a/telegram/_games/game.py +++ b/telegram/_games/game.py @@ -117,9 +117,9 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["Game"]: return super().de_json(data=data, bot=bot) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["photo"] = [p.to_dict() for p in self.photo] if self.text_entities: diff --git a/telegram/_inline/inlinekeyboardmarkup.py b/telegram/_inline/inlinekeyboardmarkup.py index 39b96da96c9..52c3982882a 100644 --- a/telegram/_inline/inlinekeyboardmarkup.py +++ b/telegram/_inline/inlinekeyboardmarkup.py @@ -68,9 +68,9 @@ def __init__( self._id_attrs = (self.inline_keyboard,) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["inline_keyboard"] = [] for inline_keyboard in self.inline_keyboard: diff --git a/telegram/_inline/inlinequeryresult.py b/telegram/_inline/inlinequeryresult.py index 60da54ad56d..7ccf3c00baa 100644 --- a/telegram/_inline/inlinequeryresult.py +++ b/telegram/_inline/inlinequeryresult.py @@ -54,9 +54,9 @@ def __init__(self, type: str, id: str, *, api_kwargs: JSONDict = None): self._id_attrs = (self.id,) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) # pylint: disable=no-member if ( diff --git a/telegram/_inline/inputinvoicemessagecontent.py b/telegram/_inline/inputinvoicemessagecontent.py index a39952ed865..fbc7f3c33ba 100644 --- a/telegram/_inline/inputinvoicemessagecontent.py +++ b/telegram/_inline/inputinvoicemessagecontent.py @@ -228,9 +228,9 @@ def __hash__(self) -> int: ) ) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["prices"] = [price.to_dict() for price in self.prices] diff --git a/telegram/_inline/inputtextmessagecontent.py b/telegram/_inline/inputtextmessagecontent.py index aac335ca1d0..71c22083005 100644 --- a/telegram/_inline/inputtextmessagecontent.py +++ b/telegram/_inline/inputtextmessagecontent.py @@ -84,9 +84,9 @@ def __init__( self._id_attrs = (self.message_text,) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) if self.entities: data["entities"] = [ce.to_dict() for ce in self.entities] diff --git a/telegram/_menubutton.py b/telegram/_menubutton.py index 0ecd6df51d9..2c28ed74234 100644 --- a/telegram/_menubutton.py +++ b/telegram/_menubutton.py @@ -161,9 +161,9 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["MenuButtonWe return super().de_json(data=data, bot=bot) # type: ignore[return-value] - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["web_app"] = self.web_app.to_dict() return data diff --git a/telegram/_message.py b/telegram/_message.py index cf43b456ac6..bc0ad6428f0 100644 --- a/telegram/_message.py +++ b/telegram/_message.py @@ -718,9 +718,9 @@ def effective_attachment( return self._effective_attachment # type: ignore[return-value] - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) # Required data["date"] = to_timestamp(self.date) diff --git a/telegram/_passport/credentials.py b/telegram/_passport/credentials.py index 2c655c628ae..f7432ec199e 100644 --- a/telegram/_passport/credentials.py +++ b/telegram/_passport/credentials.py @@ -404,9 +404,9 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["SecureValue" return super().de_json(data=data, bot=bot) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["files"] = [p.to_dict() for p in self.files] # type: ignore[union-attr] data["translation"] = [p.to_dict() for p in self.translation] # type: ignore[union-attr] @@ -450,9 +450,9 @@ class DataCredentials(_CredentialsBase): def __init__(self, data_hash: str, secret: str, *, api_kwargs: JSONDict = None): super().__init__(hash=data_hash, secret=secret, api_kwargs=api_kwargs) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) del data["file_hash"] del data["hash"] @@ -479,9 +479,9 @@ class FileCredentials(_CredentialsBase): def __init__(self, file_hash: str, secret: str, *, api_kwargs: JSONDict = None): super().__init__(hash=file_hash, secret=secret, api_kwargs=api_kwargs) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) del data["data_hash"] del data["hash"] diff --git a/telegram/_passport/encryptedpassportelement.py b/telegram/_passport/encryptedpassportelement.py index 7ba7f057ab2..54281b7b869 100644 --- a/telegram/_passport/encryptedpassportelement.py +++ b/telegram/_passport/encryptedpassportelement.py @@ -245,9 +245,9 @@ def de_json_decrypted( return super().de_json(data=data, bot=bot) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) if self.files: data["files"] = [p.to_dict() for p in self.files] diff --git a/telegram/_passport/passportdata.py b/telegram/_passport/passportdata.py index d67055d0733..fbfafce216e 100644 --- a/telegram/_passport/passportdata.py +++ b/telegram/_passport/passportdata.py @@ -81,9 +81,9 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["PassportData return super().de_json(data=data, bot=bot) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["data"] = [e.to_dict() for e in self.data] diff --git a/telegram/_payment/shippingoption.py b/telegram/_payment/shippingoption.py index 36dc8bf3e75..c21c4d53435 100644 --- a/telegram/_payment/shippingoption.py +++ b/telegram/_payment/shippingoption.py @@ -65,9 +65,9 @@ def __init__( self._id_attrs = (self.id,) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["prices"] = [p.to_dict() for p in self.prices] diff --git a/telegram/_poll.py b/telegram/_poll.py index 43c62652c5a..5ed9a1b1c73 100644 --- a/telegram/_poll.py +++ b/telegram/_poll.py @@ -228,9 +228,9 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["Poll"]: return super().de_json(data=data, bot=bot) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["options"] = [x.to_dict() for x in self.options] if self.explanation_entities: diff --git a/telegram/_replykeyboardmarkup.py b/telegram/_replykeyboardmarkup.py index 5e78bd5a1f9..c122670ee6c 100644 --- a/telegram/_replykeyboardmarkup.py +++ b/telegram/_replykeyboardmarkup.py @@ -119,9 +119,9 @@ def __init__( self._id_attrs = (self.keyboard,) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["keyboard"] = [] for row in self.keyboard: diff --git a/telegram/_userprofilephotos.py b/telegram/_userprofilephotos.py index 7a513690330..ee0e8ad3522 100644 --- a/telegram/_userprofilephotos.py +++ b/telegram/_userprofilephotos.py @@ -69,9 +69,9 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["UserProfileP return super().de_json(data=data, bot=bot) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) data["photos"] = [] for photo in self.photos: diff --git a/telegram/_videochat.py b/telegram/_videochat.py index afd7507510a..41bc25c641e 100644 --- a/telegram/_videochat.py +++ b/telegram/_videochat.py @@ -121,9 +121,9 @@ def de_json( data["users"] = User.de_list(data.get("users", []), bot) return super().de_json(data=data, bot=bot) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) if self.users is not None: data["users"] = [u.to_dict() for u in self.users] @@ -176,9 +176,9 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["VideoChatSch return super().de_json(data=data, bot=bot) - def to_dict(self) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: """See :meth:`telegram.TelegramObject.to_dict`.""" - data = super().to_dict() + data = super().to_dict(recursive=recursive) # Required data["start_date"] = to_timestamp(self.start_date) From 53fbf9ab9ccc49f57d2093b7a52b310eccfac961 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Sun, 2 Oct 2022 03:29:03 +0530 Subject: [PATCH 03/11] review: add comment to test class --- tests/test_telegramobject.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/test_telegramobject.py b/tests/test_telegramobject.py index 7c2184f7877..63972419a46 100644 --- a/tests/test_telegramobject.py +++ b/tests/test_telegramobject.py @@ -139,24 +139,26 @@ def test_to_dict_api_kwargs(self): assert to.to_dict() == {"foo": "bar"} def test_to_dict_recursion(self): - class Recusive(TelegramObject): + class Recursive(TelegramObject): + __slots__ = ("recursive",) + def __init__(self): super().__init__() self.recursive = "recursive" - class SubClass(TelegramObject): + class SubClass(TelegramObject): # doesn't have `__slots__`, so has `__dict__` instead. def __init__(self): super().__init__() - self.subclass = Recusive() + self.subclass = Recursive() to = SubClass() to_dict_no_recurse = to.to_dict(recursive=False) assert to_dict_no_recurse - assert isinstance(to_dict_no_recurse["subclass"], Recusive) - to_dict_recuse = to.to_dict(recursive=True) - assert to_dict_recuse - assert isinstance(to_dict_recuse["subclass"], dict) - assert to_dict_recuse["subclass"]["recursive"] == "recursive" + assert isinstance(to_dict_no_recurse["subclass"], Recursive) + to_dict_recurse = to.to_dict(recursive=True) + assert to_dict_recurse + assert isinstance(to_dict_recurse["subclass"], dict) + assert to_dict_recurse["subclass"]["recursive"] == "recursive" def test_slot_behaviour(self, mro_slots): inst = TelegramObject() From 61ab03219e3ae74de3517ed662657881c07a01e5 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Sun, 2 Oct 2022 22:51:00 +0530 Subject: [PATCH 04/11] review: use itertools.chain instead --- telegram/_telegramobject.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index 5b4cbe971bc..2604fe261fa 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -20,6 +20,7 @@ import inspect import json from copy import deepcopy +from itertools import chain from typing import TYPE_CHECKING, Dict, List, Optional, Set, Tuple, Type, TypeVar, Union from telegram._utils.types import JSONDict @@ -175,20 +176,19 @@ def _get_attrs( except AttributeError: pass - tuple_data = tuple(data) # We want to get all attributes for the class, using self.__slots__ only includes the # attributes used by that class itself, and not its superclass(es). Hence, we get its MRO # and then get their attributes. The `[:-1]` slice excludes the `object` class for cls in self.__class__.__mro__[:-1]: - # add the class's slots with the user defined subclass __dict__ (class has no slots) - for key in tuple(cls.__slots__) + tuple_data: # type: ignore[attr-defined] + # chain the class's slots with the user defined subclass __dict__ (class has no slots) + for key in chain(cls.__slots__, data): # type: ignore[attr-defined] if not include_private and key.startswith("_"): continue value = getattr(self, key, None) if value is not None: if recursive and hasattr(value, "to_dict"): - data[key] = value.to_dict() # pylint: disable=no-member + data[key] = value.to_dict(recursive=True) # pylint: disable=no-member else: data[key] = value elif not recursive: From bf23a7f0aa0fa6a09d2427b34e26dd15db1b479c Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Sun, 2 Oct 2022 23:25:34 +0530 Subject: [PATCH 05/11] further get_attrs optimizations make and use copy of data, since data increases length every iteration. Use generator to get all slots, and use hasattr() instead of try-except --- telegram/_telegramobject.py | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index 2604fe261fa..5968d4f0595 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -170,29 +170,28 @@ def _get_attrs( """ data = {} - try: - # __dict__ has attrs from superclasses, so no need to recompute/duplicate - data.update(self.__dict__) - except AttributeError: - pass + # __dict__ has attrs from superclasses, so no need to recompute/duplicate + if hasattr(self, "__dict__"): + data.update(self.__dict__) # important when class has no __slots__. + org_data = tuple(data) # make a copy of the keys to avoid changing length during iteration # We want to get all attributes for the class, using self.__slots__ only includes the # attributes used by that class itself, and not its superclass(es). Hence, we get its MRO # and then get their attributes. The `[:-1]` slice excludes the `object` class - for cls in self.__class__.__mro__[:-1]: - # chain the class's slots with the user defined subclass __dict__ (class has no slots) - for key in chain(cls.__slots__, data): # type: ignore[attr-defined] - if not include_private and key.startswith("_"): - continue - - value = getattr(self, key, None) - if value is not None: - if recursive and hasattr(value, "to_dict"): - data[key] = value.to_dict(recursive=True) # pylint: disable=no-member - else: - data[key] = value - elif not recursive: + all_slots = (slot for cls in self.__class__.__mro__[:-1] for slot in cls.__slots__) + # chain the class's slots with the user defined subclass __dict__ (class has no slots) + for key in chain(org_data, all_slots): + if not include_private and key.startswith("_"): + continue + + value = getattr(self, key, None) + if value is not None: + if recursive and hasattr(value, "to_dict"): + data[key] = value.to_dict(recursive=True) # pylint: disable=no-member + else: data[key] = value + elif not recursive: + data[key] = value if recursive and data.get("from_user"): data["from"] = data.pop("from_user", None) From cc98b741b24e133c2895e139de5eb42147a77b3a Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Mon, 3 Oct 2022 01:19:28 +0530 Subject: [PATCH 06/11] mypy ignore --- telegram/_telegramobject.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index 5968d4f0595..109329c730c 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -178,7 +178,7 @@ def _get_attrs( # We want to get all attributes for the class, using self.__slots__ only includes the # attributes used by that class itself, and not its superclass(es). Hence, we get its MRO # and then get their attributes. The `[:-1]` slice excludes the `object` class - all_slots = (slot for cls in self.__class__.__mro__[:-1] for slot in cls.__slots__) + all_slots = (s for c in self.__class__.__mro__[:-1] for s in c.__slots__) # type: ignore # chain the class's slots with the user defined subclass __dict__ (class has no slots) for key in chain(org_data, all_slots): if not include_private and key.startswith("_"): From bd80031646f4b3b18692c3e806b8e2de1f89b749 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Sun, 9 Oct 2022 14:14:28 +0530 Subject: [PATCH 07/11] fix doc buid --- telegram/_telegramobject.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index 109329c730c..00250971d8d 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -291,7 +291,7 @@ def to_dict(self, recursive: bool = True) -> JSONDict: (if found) in the attributes to a dictionary. Else, preserves it as an object itself. Defaults to :obj:`True`. - .. versionadded:: 20.0 + .. versionadded:: 20.0 Returns: :obj:`dict` From 8052acbb7a39a6c0b3b551b0f85e1fa01ec7c6f9 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Thu, 13 Oct 2022 20:06:44 +0530 Subject: [PATCH 08/11] review 2: org_data now not a tuple, comment reverted --- telegram/_telegramobject.py | 10 ++++++---- tests/test_telegramobject.py | 4 +++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index 00250971d8d..c08d7266fda 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -161,8 +161,8 @@ def _get_attrs( Args: include_private (:obj:`bool`): Whether the result should include private variables. - recursive (:obj:`bool`): If :obj:`True`, will convert any TelegramObjects (if found) in - the attributes to a dictionary. Else, preserves it as an object itself. + recursive (:obj:`bool`): If :obj:`True`, will convert any ``TelegramObjects`` (if + found) in the attributes to a dictionary. Else, preserves it as an object itself. remove_bot (:obj:`bool`): Whether the bot should be included in the result. Returns: @@ -170,11 +170,13 @@ def _get_attrs( """ data = {} - # __dict__ has attrs from superclasses, so no need to recompute/duplicate + # __dict__ has attrs from superclasses, so no need to loop through them if hasattr(self, "__dict__"): + org_data = self.__dict__ data.update(self.__dict__) # important when class has no __slots__. + else: + org_data = {} - org_data = tuple(data) # make a copy of the keys to avoid changing length during iteration # We want to get all attributes for the class, using self.__slots__ only includes the # attributes used by that class itself, and not its superclass(es). Hence, we get its MRO # and then get their attributes. The `[:-1]` slice excludes the `object` class diff --git a/tests/test_telegramobject.py b/tests/test_telegramobject.py index 63972419a46..5de88e6d3cc 100644 --- a/tests/test_telegramobject.py +++ b/tests/test_telegramobject.py @@ -146,7 +146,9 @@ def __init__(self): super().__init__() self.recursive = "recursive" - class SubClass(TelegramObject): # doesn't have `__slots__`, so has `__dict__` instead. + class SubClass(TelegramObject): + """This class doesn't have `__slots__`, so has `__dict__` instead.""" + def __init__(self): super().__init__() self.subclass = Recursive() From 049a8c1d18c8518010c6a2a87b77f50afc3f44ea Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Fri, 14 Oct 2022 16:56:34 +0530 Subject: [PATCH 09/11] review 2: remove org_data variable --- telegram/_telegramobject.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index c08d7266fda..91ed80e5f1d 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -172,17 +172,14 @@ def _get_attrs( # __dict__ has attrs from superclasses, so no need to loop through them if hasattr(self, "__dict__"): - org_data = self.__dict__ data.update(self.__dict__) # important when class has no __slots__. - else: - org_data = {} # We want to get all attributes for the class, using self.__slots__ only includes the # attributes used by that class itself, and not its superclass(es). Hence, we get its MRO # and then get their attributes. The `[:-1]` slice excludes the `object` class all_slots = (s for c in self.__class__.__mro__[:-1] for s in c.__slots__) # type: ignore # chain the class's slots with the user defined subclass __dict__ (class has no slots) - for key in chain(org_data, all_slots): + for key in chain(self.__dict__, all_slots) if hasattr(self, "__dict__") else all_slots: if not include_private and key.startswith("_"): continue From 7abac55c215c3c6ac20bd865fc2a6300b89d4e24 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Tue, 18 Oct 2022 00:14:00 +0530 Subject: [PATCH 10/11] review 3: optimize further by removing first `__dict__` check --- telegram/_telegramobject.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/telegram/_telegramobject.py b/telegram/_telegramobject.py index 91ed80e5f1d..a0bb9ebe146 100644 --- a/telegram/_telegramobject.py +++ b/telegram/_telegramobject.py @@ -170,10 +170,6 @@ def _get_attrs( """ data = {} - # __dict__ has attrs from superclasses, so no need to loop through them - if hasattr(self, "__dict__"): - data.update(self.__dict__) # important when class has no __slots__. - # We want to get all attributes for the class, using self.__slots__ only includes the # attributes used by that class itself, and not its superclass(es). Hence, we get its MRO # and then get their attributes. The `[:-1]` slice excludes the `object` class From e1e63fcddbe824f619a6f01c5843ad81f827a853 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Tue, 18 Oct 2022 18:57:31 +0530 Subject: [PATCH 11/11] Update telegram/_bot.py Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com> --- telegram/_bot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index df0771a15d5..a2929da28eb 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -8205,7 +8205,7 @@ async def create_invoice_link( api_kwargs=api_kwargs, ) - def to_dict(self, recursive: bool = True) -> JSONDict: + def to_dict(self, recursive: bool = True) -> JSONDict: # skipcq: PYL-W0613 """See :meth:`telegram.TelegramObject.to_dict`.""" data: JSONDict = {"id": self.id, "username": self.username, "first_name": self.first_name}