Simplify Handling of Empty Data in TelegramObject.de_json and Friends

[Bibo-Joshi]

When Ready, closes #4614

Currently, I expect that a bunch of tests will fail as I haven't updated them yet

[codecov]

❌ 177 Tests Failed:

Tests completed	Failed	Passed	Skipped
6375	177	6198	452

View the top 3 failed tests by shortest run time

tests.test_bot.TestBotWithoutRequest::test_defaults_handling[Bot.get_forum_topic_icon_stickers]

Stack Traces | 0.002s run time

self = <tests.test_bot.TestBotWithoutRequest object at 0x0000013990675270>
bot_class = <class 'telegram._bot.Bot'>
bot_method_name = 'get_forum_topic_icon_stickers'
bot_method = <bound method ExtBot.get_forum_topic_icon_stickers of PytestExtBot[token=5737018356:AAH138SuiKQF0LDCWsfgWeXfjJ5d63kCWLA]>
offline_bot = PytestExtBot[token=5737018356:AAH138SuiKQF0LDCWsfgWeXfjJ5d63kCWLA]
raw_bot = PytestBot[token=5737018356:AAH138SuiKQF0LDCWsfgWeXfjJ5d63kCWLA]

    @bot_methods(ext_bot=False)
    async def test_defaults_handling(
        self,
        bot_class,
        bot_method_name: str,
        bot_method,
        offline_bot: PytestExtBot,
        raw_bot: PytestBot,
    ):
        """
        Here we check that the offline_bot methods handle tg.ext.Defaults correctly. This has two
        parts:
    
        1. Check that ExtBot actually inserts the defaults values correctly
        2. Check that tg.Bot just replaces `DefaultValue(obj)` with `obj`, i.e. that it doesn't
            pass any `DefaultValue` instances to Request. See the docstring of
            tg.Bot._insert_defaults for details on why we need that
    
        As for most defaults,
        we can't really check the effect, we just check if we're passing the correct kwargs to
        Request.post. As offline_bot method tests a scattered across the different test files, we
        do this here in one place.
    
        The same test is also run for all the shortcuts (Message.reply_text) etc in the
        corresponding tests.
    
        Finally, there are some tests for Defaults.{parse_mode, quote, allow_sending_without_reply}
        at the appropriate places, as those are the only things we can actually check.
        """
        # Mocking get_me within check_defaults_handling messes with the cached values like
        # Bot.{bot, username, id, …}` unless we return the expected User object.
        return_value = (
            offline_bot.bot if bot_method_name.lower().replace("_", "") == "getme" else None
        )
    
        # Check that ExtBot does the right thing
        bot_method = getattr(offline_bot, bot_method_name)
        raw_bot_method = getattr(raw_bot, bot_method_name)
>       assert await check_defaults_handling(bot_method, offline_bot, return_value=return_value)

tests\test_bot.py:500: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests\auxil\bot_method_checks.py:668: in check_defaults_handling
    raise exc
tests\auxil\bot_method_checks.py:636: in check_defaults_handling
    assert await method(**kwargs) in expected_return_values
telegram\ext\_extbot.py:1913: in get_forum_topic_icon_stickers
    return await super().get_forum_topic_icon_stickers(
telegram\_bot.py:8328: in get_forum_topic_icon_stickers
    return Sticker.de_list(result, self)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

cls = <class 'telegram._files.sticker.Sticker'>, data = None
bot = PytestExtBot[token=5737018356:AAH138SuiKQF0LDCWsfgWeXfjJ5d63kCWLA]

    @classmethod
    def de_list(
        cls: type[Tele_co], data: list[JSONDict], bot: Optional["Bot"] = None
    ) -> tuple[Tele_co, ...]:
        """Converts a list of JSON objects to a tuple of Telegram objects.
    
        .. versionchanged:: 20.0
    
           * Returns a tuple instead of a list.
           * Filters out any :obj:`None` values.
    
        Args:
            data (list[dict[:obj:`str`, ...]]): The JSON data.
            bot (:class:`telegram.Bot`, optional): The bot associated with these object. Defaults
                to :obj:`None`, in which case shortcut methods will not be available.
    
                .. versionchanged:: 21.4
                   :paramref:`bot` is now optional and defaults to :obj:`None`
    
        Returns:
            A tuple of Telegram objects.
    
        """
>       return tuple(obj for obj in (cls.de_json(d, bot) for d in data))
E       TypeError: 'NoneType' object is not iterable

telegram\_telegramobject.py:457: TypeError

tests.test_bot.TestBotWithoutRequest::test_defaults_handling[Bot.get_chat]

Stack Traces | 0.003s run time

self = <tests.test_bot.TestBotWithoutRequest object at 0x0000013990674FF0>
bot_class = <class 'telegram._bot.Bot'>, bot_method_name = 'get_chat'
bot_method = <bound method ExtBot.get_chat of PytestExtBot[token=5737018356:AAH138SuiKQF0LDCWsfgWeXfjJ5d63kCWLA]>
offline_bot = PytestExtBot[token=5737018356:AAH138SuiKQF0LDCWsfgWeXfjJ5d63kCWLA]
raw_bot = PytestBot[token=5737018356:AAH138SuiKQF0LDCWsfgWeXfjJ5d63kCWLA]

    @bot_methods(ext_bot=False)
    async def test_defaults_handling(
        self,
        bot_class,
        bot_method_name: str,
        bot_method,
        offline_bot: PytestExtBot,
        raw_bot: PytestBot,
    ):
        """
        Here we check that the offline_bot methods handle tg.ext.Defaults correctly. This has two
        parts:
    
        1. Check that ExtBot actually inserts the defaults values correctly
        2. Check that tg.Bot just replaces `DefaultValue(obj)` with `obj`, i.e. that it doesn't
            pass any `DefaultValue` instances to Request. See the docstring of
            tg.Bot._insert_defaults for details on why we need that
    
        As for most defaults,
        we can't really check the effect, we just check if we're passing the correct kwargs to
        Request.post. As offline_bot method tests a scattered across the different test files, we
        do this here in one place.
    
        The same test is also run for all the shortcuts (Message.reply_text) etc in the
        corresponding tests.
    
        Finally, there are some tests for Defaults.{parse_mode, quote, allow_sending_without_reply}
        at the appropriate places, as those are the only things we can actually check.
        """
        # Mocking get_me within check_defaults_handling messes with the cached values like
        # Bot.{bot, username, id, …}` unless we return the expected User object.
        return_value = (
            offline_bot.bot if bot_method_name.lower().replace("_", "") == "getme" else None
        )
    
        # Check that ExtBot does the right thing
        bot_method = getattr(offline_bot, bot_method_name)
        raw_bot_method = getattr(raw_bot, bot_method_name)
>       assert await check_defaults_handling(bot_method, offline_bot, return_value=return_value)

tests\test_bot.py:500: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests\auxil\bot_method_checks.py:668: in check_defaults_handling
    raise exc
tests\auxil\bot_method_checks.py:636: in check_defaults_handling
    assert await method(**kwargs) in expected_return_values
telegram\ext\_extbot.py:889: in get_chat
    result = await super().get_chat(
telegram\_bot.py:4732: in get_chat
    return ChatFullInfo.de_json(result, self)
telegram\_chatfullinfo.py:517: in de_json
    data = cls._parse_data(data)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

data = None

    @staticmethod
    def _parse_data(data: JSONDict) -> JSONDict:
        """Should be called by subclasses that override de_json to ensure that the input
        is not altered. Whoever calls de_json might still want to use the original input
        for something else.
        """
>       return data.copy()
E       AttributeError: 'NoneType' object has no attribute 'copy'

telegram\_telegramobject.py:385: AttributeError

tests.test_bot.TestBotWithoutRequest::test_defaults_handling[Bot.get_star_transactions]

Stack Traces | 0.003s run time

self = <tests.test_bot.TestBotWithoutRequest object at 0x0000013990675590>
bot_class = <class 'telegram._bot.Bot'>
bot_method_name = 'get_star_transactions'
bot_method = <bound method ExtBot.get_star_transactions of PytestExtBot[token=5737018356:AAH138SuiKQF0LDCWsfgWeXfjJ5d63kCWLA]>
offline_bot = PytestExtBot[token=5737018356:AAH138SuiKQF0LDCWsfgWeXfjJ5d63kCWLA]
raw_bot = PytestBot[token=5737018356:AAH138SuiKQF0LDCWsfgWeXfjJ5d63kCWLA]

    @bot_methods(ext_bot=False)
    async def test_defaults_handling(
        self,
        bot_class,
        bot_method_name: str,
        bot_method,
        offline_bot: PytestExtBot,
        raw_bot: PytestBot,
    ):
        """
        Here we check that the offline_bot methods handle tg.ext.Defaults correctly. This has two
        parts:
    
        1. Check that ExtBot actually inserts the defaults values correctly
        2. Check that tg.Bot just replaces `DefaultValue(obj)` with `obj`, i.e. that it doesn't
            pass any `DefaultValue` instances to Request. See the docstring of
            tg.Bot._insert_defaults for details on why we need that
    
        As for most defaults,
        we can't really check the effect, we just check if we're passing the correct kwargs to
        Request.post. As offline_bot method tests a scattered across the different test files, we
        do this here in one place.
    
        The same test is also run for all the shortcuts (Message.reply_text) etc in the
        corresponding tests.
    
        Finally, there are some tests for Defaults.{parse_mode, quote, allow_sending_without_reply}
        at the appropriate places, as those are the only things we can actually check.
        """
        # Mocking get_me within check_defaults_handling messes with the cached values like
        # Bot.{bot, username, id, …}` unless we return the expected User object.
        return_value = (
            offline_bot.bot if bot_method_name.lower().replace("_", "") == "getme" else None
        )
    
        # Check that ExtBot does the right thing
        bot_method = getattr(offline_bot, bot_method_name)
        raw_bot_method = getattr(raw_bot, bot_method_name)
>       assert await check_defaults_handling(bot_method, offline_bot, return_value=return_value)

tests\test_bot.py:500: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests\auxil\bot_method_checks.py:668: in check_defaults_handling
    raise exc
tests\auxil\bot_method_checks.py:636: in check_defaults_handling
    assert await method(**kwargs) in expected_return_values
telegram\ext\_extbot.py:4309: in get_star_transactions
    return await super().get_star_transactions(
telegram\_bot.py:9405: in get_star_transactions
    return StarTransactions.de_json(
telegram\_payment\stars\startransactions.py:159: in de_json
    data = cls._parse_data(data)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

data = None

    @staticmethod
    def _parse_data(data: JSONDict) -> JSONDict:
        """Should be called by subclasses that override de_json to ensure that the input
        is not altered. Whoever calls de_json might still want to use the original input
        for something else.
        """
>       return data.copy()
E       AttributeError: 'NoneType' object has no attribute 'copy'

telegram\_telegramobject.py:385: AttributeError

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

[Bibo-Joshi]

Admittedly, this turned out to be a bigger change than expected. Not really the removal of if not data: return None itself, but all that depended on that. However, this shows how much the internal logic of de_json and our tests dependet on this undocumented implementation detail that has no user facing benefit. Clearing that up and making the business logic independent of satisfying minor convenience needs for the tests is a good change IMO.

[harshil21]

just a couple of questions-