From fa4d41217a76af42d622744fc646099bb16a0879 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Thu, 31 Aug 2023 10:54:25 +0400 Subject: [PATCH 01/15] Add type checking for method parameters (initial version) --- tests/test_official.py | 231 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 221 insertions(+), 10 deletions(-) diff --git a/tests/test_official.py b/tests/test_official.py index f39a8ec1268..19c6825220e 100644 --- a/tests/test_official.py +++ b/tests/test_official.py @@ -19,7 +19,8 @@ import inspect import os import re -from typing import Dict, List, Set +from datetime import datetime +from typing import Any, ForwardRef, Sequence, get_args, get_origin import httpx import pytest @@ -27,6 +28,7 @@ import telegram from telegram._utils.defaultvalue import DefaultValue +from telegram._utils.types import DVInput, FileInput, ODVInput, ReplyMarkup from tests.auxil.envvars import env_var_2_bool IGNORED_OBJECTS = ("ResponseParameters", "CallbackGame") @@ -62,7 +64,7 @@ } -def _get_params_base(object_name: str, search_dict: Dict[str, Set[str]]) -> Set[str]: +def _get_params_base(object_name: str, search_dict: dict[str, set[str]]) -> set[str]: """Helper function for the *_params functions below. Given an object name and a search dict, goes through the keys of the search dict and checks if the object name matches any of the regexes (keys). The union of all the sets (values) of the @@ -79,7 +81,7 @@ def _get_params_base(object_name: str, search_dict: Dict[str, Set[str]]) -> Set[ return out -def ptb_extra_params(object_name) -> Set[str]: +def ptb_extra_params(object_name) -> set[str]: return _get_params_base(object_name, PTB_EXTRA_PARAMS) @@ -96,7 +98,7 @@ def ptb_extra_params(object_name) -> Set[str]: } -def ptb_ignored_params(object_name) -> Set[str]: +def ptb_ignored_params(object_name) -> set[str]: return _get_params_base(object_name, PTB_IGNORED_PARAMS) @@ -111,7 +113,7 @@ def ptb_ignored_params(object_name) -> Set[str]: } -def ignored_param_requirements(object_name) -> Set[str]: +def ignored_param_requirements(object_name) -> set[str]: return _get_params_base(object_name, IGNORED_PARAM_REQUIREMENTS) @@ -119,7 +121,7 @@ def ignored_param_requirements(object_name) -> Set[str]: BACKWARDS_COMPAT_KWARGS = {} -def backwards_compat_kwargs(object_name: str) -> Set[str]: +def backwards_compat_kwargs(object_name: str) -> set[str]: return _get_params_base(object_name, BACKWARDS_COMPAT_KWARGS) @@ -135,7 +137,7 @@ def find_next_sibling_until(tag, name, until): return None -def parse_table(h4) -> List[List[str]]: +def parse_table(h4) -> list[list[str]]: """Parses the Telegram doc table and has an output of a 2D list.""" table = find_next_sibling_until(h4, "table", h4.find_next_sibling("h4")) if not table: @@ -145,7 +147,10 @@ def parse_table(h4) -> List[List[str]]: def check_method(h4): name = h4.text # name of the method in telegram's docs. - method = getattr(telegram.Bot, name) # Retrieve our lib method + method = getattr(telegram.Bot, name, False) # Retrieve our lib method + if not method: + raise AssertionError(f"Method {name} not found in telegram.Bot") + table = parse_table(h4) # Check arguments based on source @@ -159,7 +164,16 @@ def check_method(h4): if param is None: raise AssertionError(f"Parameter {tg_parameter[0]} not found in {method.__name__}") - # TODO: Check type via docstring + # Check if type annotation is present and correct + if param.annotation is inspect.Parameter.empty: + raise AssertionError( + f"Param {param.name!r} of {method.__name__!r} should have a type annotation" + ) + if not check_param_type(param, tg_parameter, name): + raise AssertionError( + f"Param {param.name!r} of {method.__name__!r} should be {tg_parameter[1]}" + ) + # Now check if the parameter is required or not if not check_required_param(tg_parameter, param, method.__name__): raise AssertionError( @@ -244,7 +258,7 @@ def is_parameter_required_by_tg(field: str) -> bool: def check_required_param( - param_desc: List[str], param: inspect.Parameter, method_or_obj_name: str + param_desc: list[str], param: inspect.Parameter, method_or_obj_name: str ) -> bool: """Checks if the method/class parameter is a required/optional param as per Telegram docs. @@ -264,6 +278,203 @@ def check_defaults_type(ptb_param: inspect.Parameter) -> bool: return DefaultValue.get_value(ptb_param.default) is None +def check_param_type(ptb_param: inspect.Parameter, tg_parameter: list[str], name: str) -> bool: + """This function checks whether the type annotation of the parameter is the same as the one + specified in the official API. It also checks for some special cases where we accept more types + + Args: + ptb_param (inspect.Parameter): The parameter object from our methods/classes + tg_parameter (list[str]): The table row corresponding to the parameter from official API. + name (str): The name of the method/class + + Returns: + :obj:`bool`: The boolean returned represents whether our parameter's type annotation is the + same as Telegram's or not. + """ + # In order to evaluate the type annotation, we need to first have a mapping of the types + # specified in the official API to our types. The keys are types in the column of official API. + TYPE_MAPPING = { + "Integer or String": {int | str}, + "Integer": {int}, + "String": {str}, + "Boolean": {bool}, + "Float number": {float}, + r"Array of [\w\,\s]*": {Sequence}, + r"Array of Array of [\w\,\s]*": {Sequence, Sequence}, + "InlineKeyboardMarkup or ReplyKeyboardMarkup or ReplyKeyboardRemove or ForceReply": { + ReplyMarkup + }, + r"InputFile(?: or String)?": {FileInput}, + } + + tg_param_type = tg_parameter[1] # Type of parameter as specified in the docs + mapped: set[type] = _get_params_base(tg_param_type, TYPE_MAPPING) + if len(mapped) == 0: + # We want to store both string version of class and the class obj itself. e.g. "InputMedia" + # and InputMedia because some annotations might be ForwardRefs. + mapped_type: list = [ + getattr(telegram, tg_param_type), + ForwardRef(tg_param_type), + tg_param_type, + ] + elif len(mapped) == 1: + mapped_type: set = mapped.pop() + else: + mapped_type = mapped + # print("SHOULDNOT REACH HERE") + + # Resolve nested annotations to get inner types. + if (ptb_annotation := list(get_args(ptb_param.annotation))) == []: + ptb_annotation = ptb_param.annotation # if it's not nested, just use the annotation + + # print(f"{tg_param_type=} ||| {mapped_type=} ||| {ptb_annotation=}") + + if isinstance(ptb_annotation, list): + # Some cleaning: + # Remove 'Optional[...]' from the annotation if it's present. We do it this way since: 1) + # we already check if argument should be optional or not + type checkers will complain. + # 2) we don't want to deal with Optional[..] skewing the equality check of our annotation + # with the official API, i.e. Optional[int] == int. + if type(None) in ptb_annotation: + print(f"found None in {ptb_param.name}") + ptb_annotation.remove(type(None)) + + # Cleaning done... now let's put it back together. + # Join all the annotations back (i.e. Union) + ptb_annotation = _unionizer(ptb_annotation, False) + + # Last step, we need to use get_origin to get the original type, since using get_args + # above will strip that out. + wrapped = get_origin(ptb_param.annotation) + if wrapped is not None: + # collections.abc.Sequence -> typing.Sequence + if "collections.abc.Sequence" in str(wrapped): + wrapped = Sequence + ptb_annotation = wrapped[ptb_annotation] + # We have put back our annotation together after removing the NoneType! + + # Now let's do the checking + # Loose type checking for "Array of " annotations for now. TODO: make it strict later + if "Array of " in tg_param_type: + # print("array of ", ptb_param.annotation) + + # For exceptions just check if they contain the annotation + array_of_exceptions = { + "results": "InlineQueryResult", + "commands": "BotCommand", + } + if ptb_param.name in array_of_exceptions: + return array_of_exceptions[ptb_param.name] in str(ptb_annotation) + + # let's import obj + pattern = r"Array of(?: Array of)? ([\w\,\s]*)" # matches "Array of [Array of] [obj]" + obj_str: str = re.search(pattern, tg_param_type).group(1) # extract obj from string + # is obj a regular type like str? + array_of_mapped: set[type] = _get_params_base(obj_str, TYPE_MAPPING) + + if len(array_of_mapped) == 0: # no match found, it's from telegram module + # print('from tg module') + # it could be a list of objects, so let's check that: + objs = _extract_words(obj_str) + # let's unionize all the objects + obj = _unionizer(objs, True) + else: + print("from TYPE_MAPPING") + obj = array_of_mapped.pop() + + # This means it is Array of [obj] + if isinstance(mapped_type, type) or "Sequence" in str(mapped_type): + # print('isinstance', mapped_type[obj], ptb_annotation) + return mapped_type[obj] == ptb_annotation + # This means it is Array of Array of [obj] + if len(mapped_type) == 1: + ele = mapped_type.pop() + seq_1: Sequence = ele[0] + seq_2: Sequence = ele[1] + # print('isinstance 2', seq_1[seq_2[obj]], ptb_annotation) + return seq_1[seq_2[obj]] == ptb_annotation + + DEFAULT_VAL_PARAMS = { + "parse_mode", + "disable_notification", + "allow_sending_without_reply", + "protect_content", + "disable_web_page_preview", + "explanation_parse_mode", + } + + # Special case for when the parameter is a default value parameter + if ptb_param.name in DEFAULT_VAL_PARAMS: + # Check if it's DVInput or ODVInput + for param_type in [DVInput, ODVInput]: + parsed = param_type[mapped_type] + if ptb_annotation == parsed: + return True + # print(f"cause {mapped_type} != {ptb_annotation} for {ptb_param.name}") + return False + + # Special case for send_* methods where we accept more types than the official API: + additional_types = { + "photo": ForwardRef("PhotoSize"), + "video": ForwardRef("Video"), + "video_note": ForwardRef("VideoNote"), + "audio": ForwardRef("Audio"), + "document": ForwardRef("Document"), + "animation": ForwardRef("Animation"), + "voice": ForwardRef("Voice"), + "sticker": ForwardRef("Sticker"), + } + + if ( + ptb_param.name in additional_types + and not isinstance(mapped_type, list) + and name.startswith("send") + ): + print("additional_types ") + mapped_type = mapped_type | additional_types[ptb_param.name] + + # Special cases for other methods that accept more types than the official API + exceptions = { # param_name: reduced form of annotation + "correct_option_id": int, + "file_id": str, + "invite_link": str, + "provider_data": str, + } + + if ptb_param.name in exceptions: + ptb_annotation = exceptions[ptb_param.name] + + # Special case for datetimes + if re.search(r"([_]+|\b)date[^\w]*\b", ptb_param.name): + mapped_type = mapped_type | datetime + + # Final check for the basic types + if isinstance(mapped_type, list) and any(ptb_annotation == t for t in mapped_type): + return True + + if mapped_type != ptb_annotation: + # print(f"cause {mapped_type} != {ptb_annotation} for {ptb_param.name}") + return False + + return True + + +def _extract_words(text: str) -> set[str]: + """Extracts all words from a string, removing all punctuation and words like 'and'.""" + return set(re.sub(r"[^\w\s]", "", text).split()) - {"and"} + + +def _unionizer(annotation: Sequence, forward_ref: bool) -> Any: + """Returns a union of all the types in the annotation. If forward_ref is True, it wraps the + annotation in a ForwardRef and then unionizes.""" + union = None + for t in annotation: + if forward_ref: + t = ForwardRef(t) # noqa: PLW2901 + union = t if union is None else union | t + return union + + to_run = env_var_2_bool(os.getenv("TEST_OFFICIAL")) argvalues = [] names = [] From 0d58c76550e5ffb76d2fbb589a545e495fff1853 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Thu, 31 Aug 2023 10:55:46 +0400 Subject: [PATCH 02/15] Make type hint of user_id as int only few other minor changes --- telegram/_bot.py | 52 ++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index b64768409e7..f3cff6f13f5 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -69,7 +69,6 @@ from telegram._files.document import Document from telegram._files.file import File from telegram._files.inputmedia import InputMedia -from telegram._files.inputsticker import InputSticker from telegram._files.location import Location from telegram._files.photosize import PhotoSize from telegram._files.sticker import MaskPosition, Sticker, StickerSet @@ -84,8 +83,6 @@ from telegram._menubutton import MenuButton from telegram._message import Message from telegram._messageid import MessageId -from telegram._passport.passportelementerrors import PassportElementError -from telegram._payment.shippingoption import ShippingOption from telegram._poll import Poll from telegram._sentwebappmessage import SentWebAppMessage from telegram._telegramobject import TelegramObject @@ -121,8 +118,11 @@ InputMediaDocument, InputMediaPhoto, InputMediaVideo, + InputSticker, LabeledPrice, MessageEntity, + PassportElementError, + ShippingOption, ) BT = TypeVar("BT", bound="Bot") @@ -2521,7 +2521,7 @@ async def send_contact( @_log async def send_game( self, - chat_id: Union[int, str], + chat_id: int, game_short_name: str, disable_notification: DVInput[bool] = DEFAULT_NONE, reply_to_message_id: Optional[int] = None, @@ -2539,7 +2539,7 @@ async def send_game( """Use this method to send a game. Args: - chat_id (:obj:`int` | :obj:`str`): Unique identifier for the target chat. + chat_id (:obj:`int`): Unique identifier for the target chat. game_short_name (:obj:`str`): Short name of the game, serves as the unique identifier for the game. Set up your games via `@BotFather `_. disable_notification (:obj:`bool`, optional): |disable_notification| @@ -2826,7 +2826,7 @@ async def answer_inline_query( @_log async def get_user_profile_photos( self, - user_id: Union[str, int], + user_id: int, offset: Optional[int] = None, limit: Optional[int] = None, *, @@ -2938,7 +2938,7 @@ async def get_file( async def ban_chat_member( self, chat_id: Union[str, int], - user_id: Union[str, int], + user_id: int, until_date: Optional[Union[int, datetime]] = None, revoke_messages: Optional[bool] = None, *, @@ -3046,7 +3046,7 @@ async def ban_chat_sender_chat( async def unban_chat_member( self, chat_id: Union[str, int], - user_id: Union[str, int], + user_id: int, only_if_banned: Optional[bool] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, @@ -3412,7 +3412,7 @@ async def edit_message_reply_markup( chat_id: Optional[Union[str, int]] = None, message_id: Optional[int] = None, inline_message_id: Optional[str] = None, - reply_markup: Optional["InlineKeyboardMarkup"] = None, + reply_markup: Optional[InlineKeyboardMarkup] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -3876,7 +3876,7 @@ async def get_chat_member_count( async def get_chat_member( self, chat_id: Union[str, int], - user_id: Union[str, int], + user_id: int, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -4011,9 +4011,9 @@ async def get_webhook_info( @_log async def set_game_score( self, - user_id: Union[int, str], + user_id: int, score: int, - chat_id: Optional[Union[str, int]] = None, + chat_id: Optional[int] = None, message_id: Optional[int] = None, inline_message_id: Optional[str] = None, force: Optional[bool] = None, @@ -4037,7 +4037,7 @@ async def set_game_score( decrease. This can be useful when fixing mistakes or banning cheaters. disable_edit_message (:obj:`bool`, optional): Pass :obj:`True`, if the game message should not be automatically edited to include the current scoreboard. - chat_id (:obj:`int` | :obj:`str`, optional): Required if :paramref:`inline_message_id` + chat_id (:obj:`int`, optional): Required if :paramref:`inline_message_id` is not specified. Unique identifier for the target chat. message_id (:obj:`int`, optional): Required if :paramref:`inline_message_id` is not specified. Identifier of the sent message. @@ -4076,8 +4076,8 @@ async def set_game_score( @_log async def get_game_high_scores( self, - user_id: Union[int, str], - chat_id: Optional[Union[str, int]] = None, + user_id: int, + chat_id: Optional[int] = None, message_id: Optional[int] = None, inline_message_id: Optional[str] = None, *, @@ -4101,7 +4101,7 @@ async def get_game_high_scores( Args: user_id (:obj:`int`): Target user id. - chat_id (:obj:`int` | :obj:`str`, optional): Required if :paramref:`inline_message_id` + chat_id (:obj:`int`, optional): Required if :paramref:`inline_message_id` is not specified. Unique identifier for the target chat. message_id (:obj:`int`, optional): Required if :paramref:`inline_message_id` is not specified. Identifier of the sent message. @@ -4321,7 +4321,7 @@ async def answer_shipping_query( # pylint: disable=invalid-name self, shipping_query_id: str, ok: bool, - shipping_options: Optional[Sequence[ShippingOption]] = None, + shipping_options: Optional[Sequence["ShippingOption"]] = None, error_message: Optional[str] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, @@ -4483,7 +4483,7 @@ async def answer_web_app_query( async def restrict_chat_member( self, chat_id: Union[str, int], - user_id: Union[str, int], + user_id: int, permissions: ChatPermissions, until_date: Optional[Union[int, datetime]] = None, use_independent_chat_permissions: Optional[bool] = None, @@ -4557,7 +4557,7 @@ async def restrict_chat_member( async def promote_chat_member( self, chat_id: Union[str, int], - user_id: Union[str, int], + user_id: int, can_change_info: Optional[bool] = None, can_post_messages: Optional[bool] = None, can_edit_messages: Optional[bool] = None, @@ -4723,7 +4723,7 @@ async def set_chat_permissions( async def set_chat_administrator_custom_title( self, chat_id: Union[int, str], - user_id: Union[int, str], + user_id: int, custom_title: str, *, read_timeout: ODVInput[float] = DEFAULT_NONE, @@ -5478,7 +5478,7 @@ async def get_custom_emoji_stickers( @_log async def upload_sticker_file( self, - user_id: Union[str, int], + user_id: int, sticker: Optional[FileInput], sticker_format: Optional[str], *, @@ -5538,7 +5538,7 @@ async def upload_sticker_file( @_log async def add_sticker_to_set( self, - user_id: Union[str, int], + user_id: int, name: str, sticker: Optional[InputSticker], *, @@ -5636,7 +5636,7 @@ async def set_sticker_position_in_set( @_log async def create_new_sticker_set( self, - user_id: Union[str, int], + user_id: int, name: str, title: str, stickers: Optional[Sequence[InputSticker]], @@ -5807,7 +5807,7 @@ async def delete_sticker_set( async def set_sticker_set_thumbnail( self, name: str, - user_id: Union[str, int], + user_id: int, thumbnail: Optional[FileInput] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, @@ -6079,8 +6079,8 @@ async def set_custom_emoji_sticker_set_thumbnail( @_log async def set_passport_data_errors( self, - user_id: Union[str, int], - errors: Sequence[PassportElementError], + user_id: int, + errors: Sequence["PassportElementError"], *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, From 9bec96dd91fb3f973d76b3697426e6f92e9fbe40 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Thu, 31 Aug 2023 10:56:47 +0400 Subject: [PATCH 03/15] update tests readme to tell python version needed to run test_official --- tests/README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/README.rst b/tests/README.rst index 33176f647fa..0e0808a5c1b 100644 --- a/tests/README.rst +++ b/tests/README.rst @@ -72,7 +72,7 @@ complete and correct. To run it, export an environment variable first: $ export TEST_OFFICIAL=true -and then run ``pytest tests/test_official.py``. +and then run ``pytest tests/test_official.py``. Note: You need py 3.11+ to run this test. We also have another marker, ``@pytest.mark.dev``, which you can add to tests that you want to run selectively. Use as follows: From 3ed9e5dda4c03b9103a60d1a331b66ccdb6b75c7 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Mon, 4 Sep 2023 16:44:02 +0400 Subject: [PATCH 04/15] Add type checking of parameters for classes --- tests/test_official.py | 185 +++++++++++++++++++++++------------------ 1 file changed, 106 insertions(+), 79 deletions(-) diff --git a/tests/test_official.py b/tests/test_official.py index 19c6825220e..e8fbd4d72b9 100644 --- a/tests/test_official.py +++ b/tests/test_official.py @@ -20,15 +20,16 @@ import os import re from datetime import datetime -from typing import Any, ForwardRef, Sequence, get_args, get_origin +from types import FunctionType +from typing import Any, Callable, ForwardRef, Sequence, get_args, get_origin import httpx import pytest -from bs4 import BeautifulSoup +from bs4 import BeautifulSoup, PageElement, Tag import telegram from telegram._utils.defaultvalue import DefaultValue -from telegram._utils.types import DVInput, FileInput, ODVInput, ReplyMarkup +from telegram._utils.types import DVInput, FileInput, ODVInput from tests.auxil.envvars import env_var_2_bool IGNORED_OBJECTS = ("ResponseParameters", "CallbackGame") @@ -64,7 +65,7 @@ } -def _get_params_base(object_name: str, search_dict: dict[str, set[str]]) -> set[str]: +def _get_params_base(object_name: str, search_dict: dict[str, set[Any]]) -> set[Any]: """Helper function for the *_params functions below. Given an object name and a search dict, goes through the keys of the search dict and checks if the object name matches any of the regexes (keys). The union of all the sets (values) of the @@ -81,7 +82,7 @@ def _get_params_base(object_name: str, search_dict: dict[str, set[str]]) -> set[ return out -def ptb_extra_params(object_name) -> set[str]: +def ptb_extra_params(object_name: str) -> set[str]: return _get_params_base(object_name, PTB_EXTRA_PARAMS) @@ -98,7 +99,7 @@ def ptb_extra_params(object_name) -> set[str]: } -def ptb_ignored_params(object_name) -> set[str]: +def ptb_ignored_params(object_name: str) -> set[str]: return _get_params_base(object_name, PTB_IGNORED_PARAMS) @@ -113,12 +114,12 @@ def ptb_ignored_params(object_name) -> set[str]: } -def ignored_param_requirements(object_name) -> set[str]: +def ignored_param_requirements(object_name: str) -> set[str]: return _get_params_base(object_name, IGNORED_PARAM_REQUIREMENTS) # Arguments that are optional arguments for now for backwards compatibility -BACKWARDS_COMPAT_KWARGS = {} +BACKWARDS_COMPAT_KWARGS: dict[str, set[str]] = {} def backwards_compat_kwargs(object_name: str) -> set[str]: @@ -128,7 +129,7 @@ def backwards_compat_kwargs(object_name: str) -> set[str]: IGNORED_PARAM_REQUIREMENTS.update(BACKWARDS_COMPAT_KWARGS) -def find_next_sibling_until(tag, name, until): +def find_next_sibling_until(tag: Tag, name: str, until: Tag) -> PageElement | None: for sibling in tag.next_siblings: if sibling is until: return None @@ -137,7 +138,7 @@ def find_next_sibling_until(tag, name, until): return None -def parse_table(h4) -> list[list[str]]: +def parse_table(h4: Tag) -> list[list[str]]: """Parses the Telegram doc table and has an output of a 2D list.""" table = find_next_sibling_until(h4, "table", h4.find_next_sibling("h4")) if not table: @@ -145,9 +146,9 @@ def parse_table(h4) -> list[list[str]]: return [[td.text for td in tr.find_all("td")] for tr in table.find_all("tr")[1:]] -def check_method(h4): +def check_method(h4: Tag) -> None: name = h4.text # name of the method in telegram's docs. - method = getattr(telegram.Bot, name, False) # Retrieve our lib method + method: FunctionType | None = getattr(telegram.Bot, name, None) # Retrieve our lib method if not method: raise AssertionError(f"Method {name} not found in telegram.Bot") @@ -169,7 +170,7 @@ def check_method(h4): raise AssertionError( f"Param {param.name!r} of {method.__name__!r} should have a type annotation" ) - if not check_param_type(param, tg_parameter, name): + if not check_param_type(param, tg_parameter, method): raise AssertionError( f"Param {param.name!r} of {method.__name__!r} should be {tg_parameter[1]}" ) @@ -209,7 +210,7 @@ def check_method(h4): ) -def check_object(h4): +def check_object(h4: Tag) -> None: name = h4.text obj = getattr(telegram, name) table = parse_table(h4) @@ -231,7 +232,15 @@ def check_object(h4): param = sig.parameters.get(field) if param is None: raise AssertionError(f"Attribute {field} not found in {obj.__name__}") - # TODO: Check type via docstring + # Check if type annotation is present and correct + if param.annotation is inspect.Parameter.empty: + raise AssertionError( + f"Param {param.name!r} of {obj.__name__!r} should have a type annotation" + ) + if not check_param_type(param, tg_parameter, obj): + raise AssertionError( + f"Param {param.name!r} of {obj.__name__!r} should be {tg_parameter[1]}" + ) if not check_required_param(tg_parameter, param, obj.__name__): raise AssertionError(f"{obj.__name__!r} parameter {param.name!r} requirement mismatch") @@ -278,14 +287,16 @@ def check_defaults_type(ptb_param: inspect.Parameter) -> bool: return DefaultValue.get_value(ptb_param.default) is None -def check_param_type(ptb_param: inspect.Parameter, tg_parameter: list[str], name: str) -> bool: +def check_param_type( + ptb_param: inspect.Parameter, tg_parameter: list[str], obj: FunctionType | type +) -> bool: """This function checks whether the type annotation of the parameter is the same as the one specified in the official API. It also checks for some special cases where we accept more types Args: ptb_param (inspect.Parameter): The parameter object from our methods/classes tg_parameter (list[str]): The table row corresponding to the parameter from official API. - name (str): The name of the method/class + obj (object): The object (method/class) that we are checking. Returns: :obj:`bool`: The boolean returned represents whether our parameter's type annotation is the @@ -293,41 +304,43 @@ def check_param_type(ptb_param: inspect.Parameter, tg_parameter: list[str], name """ # In order to evaluate the type annotation, we need to first have a mapping of the types # specified in the official API to our types. The keys are types in the column of official API. - TYPE_MAPPING = { + TYPE_MAPPING: dict[str, set[Any]] = { "Integer or String": {int | str}, "Integer": {int}, "String": {str}, - "Boolean": {bool}, - "Float number": {float}, - r"Array of [\w\,\s]*": {Sequence}, - r"Array of Array of [\w\,\s]*": {Sequence, Sequence}, - "InlineKeyboardMarkup or ReplyKeyboardMarkup or ReplyKeyboardRemove or ForceReply": { - ReplyMarkup - }, + r"Boolean|True": {bool}, + r"Float(?: number)?": {float}, + r"Array of (?:Array of )?[\w\,\s]*": {Sequence}, r"InputFile(?: or String)?": {FileInput}, } - tg_param_type = tg_parameter[1] # Type of parameter as specified in the docs + tg_param_type: str = tg_parameter[1] # Type of parameter as specified in the docs + is_class = inspect.isclass(obj) mapped: set[type] = _get_params_base(tg_param_type, TYPE_MAPPING) - if len(mapped) == 0: + + if not mapped: # no match found, it's from telegram module + # it could be a list of objects, so let's check that: + objs = _extract_words(tg_param_type) # We want to store both string version of class and the class obj itself. e.g. "InputMedia" # and InputMedia because some annotations might be ForwardRefs. - mapped_type: list = [ - getattr(telegram, tg_param_type), - ForwardRef(tg_param_type), - tg_param_type, - ] + if len(objs) >= 2: # We have to unionize the objects + mapped_type: tuple[Any, ...] = (_unionizer(objs, False), _unionizer(objs, True)) + else: + mapped_type = ( + getattr(telegram, tg_param_type), # This will fail if it's not from telegram mod + ForwardRef(tg_param_type), + tg_param_type, # for some reason, some annotations are not ForwardRefs, i.e. str. + ) + print("len mapped 0 ", mapped_type) elif len(mapped) == 1: - mapped_type: set = mapped.pop() - else: - mapped_type = mapped - # print("SHOULDNOT REACH HERE") + print("len mapped 1 ", mapped) + mapped_type = mapped.pop() # Resolve nested annotations to get inner types. if (ptb_annotation := list(get_args(ptb_param.annotation))) == []: ptb_annotation = ptb_param.annotation # if it's not nested, just use the annotation - # print(f"{tg_param_type=} ||| {mapped_type=} ||| {ptb_annotation=}") + print(f"{tg_param_type=} ||| {mapped_type=} ||| {ptb_annotation=}") if isinstance(ptb_annotation, list): # Some cleaning: @@ -342,6 +355,7 @@ def check_param_type(ptb_param: inspect.Parameter, tg_parameter: list[str], name # Cleaning done... now let's put it back together. # Join all the annotations back (i.e. Union) ptb_annotation = _unionizer(ptb_annotation, False) + print(ptb_annotation, "after unionizer") # Last step, we need to use get_origin to get the original type, since using get_args # above will strip that out. @@ -351,48 +365,50 @@ def check_param_type(ptb_param: inspect.Parameter, tg_parameter: list[str], name if "collections.abc.Sequence" in str(wrapped): wrapped = Sequence ptb_annotation = wrapped[ptb_annotation] + print(wrapped, "after wrapped", ptb_annotation) # We have put back our annotation together after removing the NoneType! - # Now let's do the checking - # Loose type checking for "Array of " annotations for now. TODO: make it strict later + # Now let's do the checking, starting with "Array of ..." types. if "Array of " in tg_param_type: - # print("array of ", ptb_param.annotation) + print("array of ", ptb_param.annotation) # For exceptions just check if they contain the annotation - array_of_exceptions = { + array_of_exceptions = { # We accept more types than the official API "results": "InlineQueryResult", "commands": "BotCommand", + "keyboard": "KeyboardButton", } if ptb_param.name in array_of_exceptions: return array_of_exceptions[ptb_param.name] in str(ptb_annotation) # let's import obj - pattern = r"Array of(?: Array of)? ([\w\,\s]*)" # matches "Array of [Array of] [obj]" - obj_str: str = re.search(pattern, tg_param_type).group(1) # extract obj from string + pattern = r"Array of(?: Array of)? ([\w\,\s]*)" + obj_match: re.Match | None = re.search(pattern, tg_param_type) # extract obj from string + if obj_match is None: + raise AssertionError(f"Array of {tg_param_type} not found in {ptb_param.name}") + obj_str: str = obj_match.group(1) # is obj a regular type like str? array_of_mapped: set[type] = _get_params_base(obj_str, TYPE_MAPPING) if len(array_of_mapped) == 0: # no match found, it's from telegram module - # print('from tg module') + print("from tg module") # it could be a list of objects, so let's check that: objs = _extract_words(obj_str) - # let's unionize all the objects - obj = _unionizer(objs, True) + # let's unionize all the objects, with and without ForwardRefs. + unionized_objs: list[type] = [_unionizer(objs, True), _unionizer(objs, False)] else: print("from TYPE_MAPPING") - obj = array_of_mapped.pop() + unionized_objs = [array_of_mapped.pop()] - # This means it is Array of [obj] - if isinstance(mapped_type, type) or "Sequence" in str(mapped_type): - # print('isinstance', mapped_type[obj], ptb_annotation) - return mapped_type[obj] == ptb_annotation + assert mapped_type is Sequence # This means it is Array of Array of [obj] - if len(mapped_type) == 1: - ele = mapped_type.pop() - seq_1: Sequence = ele[0] - seq_2: Sequence = ele[1] - # print('isinstance 2', seq_1[seq_2[obj]], ptb_annotation) - return seq_1[seq_2[obj]] == ptb_annotation + if "Array of Array of" in tg_param_type: + # print('isinstance 1', Sequence[Sequence[obj]], ptb_annotation) + return any(Sequence[Sequence[o]] == ptb_annotation for o in unionized_objs) + + # This means it is Array of [obj] + # print('isinstance 2', mapped_type[obj], ptb_annotation) + return any(mapped_type[o] == ptb_annotation for o in unionized_objs) DEFAULT_VAL_PARAMS = { "parse_mode", @@ -427,57 +443,66 @@ def check_param_type(ptb_param: inspect.Parameter, tg_parameter: list[str], name if ( ptb_param.name in additional_types - and not isinstance(mapped_type, list) - and name.startswith("send") + and not isinstance(mapped_type, tuple) + and obj.__name__.startswith("send") ): print("additional_types ") mapped_type = mapped_type | additional_types[ptb_param.name] - # Special cases for other methods that accept more types than the official API - exceptions = { # param_name: reduced form of annotation - "correct_option_id": int, - "file_id": str, - "invite_link": str, - "provider_data": str, + # Special cases for other methods that accept more types than the official API, and are + # too complex to compare/predict with official API: + exceptions = { # (param_name, is_class): reduced form of annotation + ("correct_option_id", False): int, # actual: Literal + ("file_id", False): str, # actual: Union[str, objs_with_file_id_attr] + ("invite_link", False): str, # actual: Union[str, ChatInviteLink] + ("provider_data", False): str, # actual: Union[str, obj] + ("callback_data", True): str, # actual: Union[str, obj] + ("media", True): str, # actual: Union[str, InputMedia*, FileInput] + ("data", True): str, # actual: Union[IdDocumentData, PersonalDetails, ResidentialAddress] } - if ptb_param.name in exceptions: - ptb_annotation = exceptions[ptb_param.name] + for (param_name, expected_class), exception_type in exceptions.items(): + if ptb_param.name == param_name and is_class is expected_class: + ptb_annotation = exception_type # Special case for datetimes if re.search(r"([_]+|\b)date[^\w]*\b", ptb_param.name): - mapped_type = mapped_type | datetime + print("datetime found") + # If it's a class, we only accept datetime as the parameter + mapped_type = datetime if is_class else mapped_type | datetime # Final check for the basic types - if isinstance(mapped_type, list) and any(ptb_annotation == t for t in mapped_type): + if isinstance(mapped_type, tuple) and any(ptb_annotation == t for t in mapped_type): return True - if mapped_type != ptb_annotation: - # print(f"cause {mapped_type} != {ptb_annotation} for {ptb_param.name}") - return False + if mapped_type == ptb_annotation: + return True - return True + print(f"cause {mapped_type=} != {ptb_annotation=} for {ptb_param.name}") + return False def _extract_words(text: str) -> set[str]: - """Extracts all words from a string, removing all punctuation and words like 'and'.""" - return set(re.sub(r"[^\w\s]", "", text).split()) - {"and"} + """Extracts all words from a string, removing all punctuation and words like 'and' & 'or'.""" + return set(re.sub(r"[^\w\s]", "", text).split()) - {"and", "or"} -def _unionizer(annotation: Sequence, forward_ref: bool) -> Any: +def _unionizer(annotation: Sequence[Any] | set[Any], forward_ref: bool) -> Any: """Returns a union of all the types in the annotation. If forward_ref is True, it wraps the annotation in a ForwardRef and then unionizes.""" union = None for t in annotation: if forward_ref: t = ForwardRef(t) # noqa: PLW2901 + elif not forward_ref and isinstance(t, str): # we have to import objects from lib + t = getattr(telegram, t) # noqa: PLW2901 union = t if union is None else union | t return union to_run = env_var_2_bool(os.getenv("TEST_OFFICIAL")) -argvalues = [] -names = [] +argvalues: list[tuple[Callable[[Tag], None], Tag]] = [] +names: list[str] = [] if to_run: argvalues = [] @@ -489,8 +514,10 @@ def _unionizer(annotation: Sequence, forward_ref: bool) -> Any: # Methods and types don't have spaces in them, luckily all other sections of the docs do # TODO: don't depend on that if "-" not in thing["name"]: - h4 = thing.parent + h4: Tag | None = thing.parent + if h4 is None: + raise AssertionError("h4 is None") # Is it a method if h4.text[0].lower() == h4.text[0]: argvalues.append((check_method, h4)) From 02b5060c080a1dd107dd01504d6cfec901b041e4 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Mon, 4 Sep 2023 16:54:12 +0400 Subject: [PATCH 05/15] Correct type hints found by new checker Breaking change: PassportFile.file_date now returns a datetime instead of integer --- telegram/_loginurl.py | 4 +-- .../_passport/encryptedpassportelement.py | 10 +++---- telegram/_passport/passportelementerrors.py | 29 +++++++++++++------ telegram/_passport/passportfile.py | 15 ++++++---- telegram/_payment/orderinfo.py | 4 +-- 5 files changed, 38 insertions(+), 24 deletions(-) diff --git a/telegram/_loginurl.py b/telegram/_loginurl.py index a8a05a07ec6..c78bc4aba0a 100644 --- a/telegram/_loginurl.py +++ b/telegram/_loginurl.py @@ -86,7 +86,7 @@ class LoginUrl(TelegramObject): def __init__( self, url: str, - forward_text: Optional[bool] = None, + forward_text: Optional[str] = None, bot_username: Optional[str] = None, request_write_access: Optional[bool] = None, *, @@ -96,7 +96,7 @@ def __init__( # Required self.url: str = url # Optional - self.forward_text: Optional[bool] = forward_text + self.forward_text: Optional[str] = forward_text self.bot_username: Optional[str] = bot_username self.request_write_access: Optional[bool] = request_write_access diff --git a/telegram/_passport/encryptedpassportelement.py b/telegram/_passport/encryptedpassportelement.py index d680e3686da..645d2d7640e 100644 --- a/telegram/_passport/encryptedpassportelement.py +++ b/telegram/_passport/encryptedpassportelement.py @@ -18,7 +18,7 @@ # along with this program. If not, see [http://www.gnu.org/licenses/]. """This module contains an object that represents a Telegram EncryptedPassportElement.""" from base64 import b64decode -from typing import TYPE_CHECKING, Optional, Sequence, Tuple +from typing import TYPE_CHECKING, Optional, Sequence, Tuple, Union from telegram._passport.credentials import decrypt_json from telegram._passport.data import IdDocumentData, PersonalDetails, ResidentialAddress @@ -54,7 +54,7 @@ class EncryptedPassportElement(TelegramObject): data (:class:`telegram.PersonalDetails` | :class:`telegram.IdDocumentData` | \ :class:`telegram.ResidentialAddress` | :obj:`str`, optional): Decrypted or encrypted data, available for "personal_details", "passport", - "driver_license", "identity_card", "identity_passport" and "address" types. + "driver_license", "identity_card", "internal_passport" and "address" types. phone_number (:obj:`str`, optional): User's verified phone number, available only for "phone_number" type. email (:obj:`str`, optional): User's verified email address, available only for "email" @@ -96,7 +96,7 @@ class EncryptedPassportElement(TelegramObject): data (:class:`telegram.PersonalDetails` | :class:`telegram.IdDocumentData` | \ :class:`telegram.ResidentialAddress` | :obj:`str`): Optional. Decrypted or encrypted data, available for "personal_details", "passport", - "driver_license", "identity_card", "identity_passport" and "address" types. + "driver_license", "identity_card", "internal_passport" and "address" types. phone_number (:obj:`str`): Optional. User's verified phone number, available only for "phone_number" type. email (:obj:`str`): Optional. User's verified email address, available only for "email" @@ -151,7 +151,7 @@ def __init__( self, type: str, # pylint: disable=redefined-builtin hash: str, # pylint: disable=redefined-builtin - data: Optional[PersonalDetails] = None, + data: Optional[Union[PersonalDetails, IdDocumentData, ResidentialAddress]] = None, phone_number: Optional[str] = None, email: Optional[str] = None, files: Optional[Sequence[PassportFile]] = None, @@ -168,7 +168,7 @@ def __init__( # Required self.type: str = type # Optionals - self.data: Optional[PersonalDetails] = data + self.data: Optional[Union[PersonalDetails, IdDocumentData, ResidentialAddress]] = data self.phone_number: Optional[str] = phone_number self.email: Optional[str] = email self.files: Tuple[PassportFile, ...] = parse_sequence_arg(files) diff --git a/telegram/_passport/passportelementerrors.py b/telegram/_passport/passportelementerrors.py index 96fd9322795..a0b5432dc93 100644 --- a/telegram/_passport/passportelementerrors.py +++ b/telegram/_passport/passportelementerrors.py @@ -19,9 +19,10 @@ # pylint: disable=redefined-builtin """This module contains the classes that represent Telegram PassportElementError.""" -from typing import Optional +from typing import Optional, Sequence, Tuple from telegram._telegramobject import TelegramObject +from telegram._utils.argumentparsing import parse_sequence_arg from telegram._utils.types import JSONDict @@ -166,14 +167,14 @@ class PassportElementErrorFiles(PassportElementError): type (:obj:`str`): The section of the user's Telegram Passport which has the issue, one of ``"utility_bill"``, ``"bank_statement"``, ``"rental_agreement"``, ``"passport_registration"``, ``"temporary_registration"``. - file_hashes (List[:obj:`str`]): List of base64-encoded file hashes. + file_hashes (Sequence[:obj:`str`]): List of base64-encoded file hashes. message (:obj:`str`): Error message. Attributes: type (:obj:`str`): The section of the user's Telegram Passport which has the issue, one of ``"utility_bill"``, ``"bank_statement"``, ``"rental_agreement"``, ``"passport_registration"``, ``"temporary_registration"``. - file_hashes (List[:obj:`str`]): List of base64-encoded file hashes. + file_hashes (Tuple[:obj:`str`]): List of base64-encoded file hashes. message (:obj:`str`): Error message. """ @@ -181,12 +182,17 @@ class PassportElementErrorFiles(PassportElementError): __slots__ = ("file_hashes",) def __init__( - self, type: str, file_hashes: str, message: str, *, api_kwargs: Optional[JSONDict] = None + self, + type: str, + file_hashes: Sequence[str], + message: str, + *, + api_kwargs: Optional[JSONDict] = None, ): # Required super().__init__("files", type, message, api_kwargs=api_kwargs) with self._unfrozen(): - self.file_hashes: str = file_hashes + self.file_hashes: Tuple[str, ...] = parse_sequence_arg(file_hashes) self._id_attrs = (self.source, self.type, self.message, *tuple(file_hashes)) @@ -357,7 +363,7 @@ class PassportElementErrorTranslationFiles(PassportElementError): one of ``"passport"``, ``"driver_license"``, ``"identity_card"``, ``"internal_passport"``, ``"utility_bill"``, ``"bank_statement"``, ``"rental_agreement"``, ``"passport_registration"``, ``"temporary_registration"``. - file_hashes (List[:obj:`str`]): List of base64-encoded file hashes. + file_hashes (Sequence[:obj:`str`]): List of base64-encoded file hashes. message (:obj:`str`): Error message. Attributes: @@ -365,7 +371,7 @@ class PassportElementErrorTranslationFiles(PassportElementError): one of ``"passport"``, ``"driver_license"``, ``"identity_card"``, ``"internal_passport"``, ``"utility_bill"``, ``"bank_statement"``, ``"rental_agreement"``, ``"passport_registration"``, ``"temporary_registration"``. - file_hashes (List[:obj:`str`]): List of base64-encoded file hashes. + file_hashes (Tuple[:obj:`str`]): List of base64-encoded file hashes. message (:obj:`str`): Error message. """ @@ -373,12 +379,17 @@ class PassportElementErrorTranslationFiles(PassportElementError): __slots__ = ("file_hashes",) def __init__( - self, type: str, file_hashes: str, message: str, *, api_kwargs: Optional[JSONDict] = None + self, + type: str, + file_hashes: Sequence[str], + message: str, + *, + api_kwargs: Optional[JSONDict] = None, ): # Required super().__init__("translation_files", type, message, api_kwargs=api_kwargs) with self._unfrozen(): - self.file_hashes: str = file_hashes + self.file_hashes: Tuple[str, ...] = parse_sequence_arg(file_hashes) self._id_attrs = (self.source, self.type, self.message, *tuple(file_hashes)) diff --git a/telegram/_passport/passportfile.py b/telegram/_passport/passportfile.py index 5d12838e0f6..fa6b094c2d3 100644 --- a/telegram/_passport/passportfile.py +++ b/telegram/_passport/passportfile.py @@ -18,9 +18,11 @@ # along with this program. If not, see [http://www.gnu.org/licenses/]. """This module contains an object that represents a Encrypted PassportFile.""" +from datetime import datetime from typing import TYPE_CHECKING, List, Optional, Tuple from telegram._telegramobject import TelegramObject +from telegram._utils.datetime import extract_tzinfo_from_defaults, from_timestamp from telegram._utils.defaultvalue import DEFAULT_NONE from telegram._utils.types import JSONDict, ODVInput @@ -43,7 +45,7 @@ class PassportFile(TelegramObject): is supposed to be the same over time and for different bots. Can't be used to download or reuse the file. file_size (:obj:`int`): File size in bytes. - file_date (:obj:`int`): Unix time when the file was uploaded. + file_date (:class:`datetime.datetime`): Datetime when the file was uploaded. Attributes: file_id (:obj:`str`): Identifier for this file, which can be used to download @@ -52,9 +54,7 @@ class PassportFile(TelegramObject): is supposed to be the same over time and for different bots. Can't be used to download or reuse the file. file_size (:obj:`int`): File size in bytes. - file_date (:obj:`int`): Unix time when the file was uploaded. - - + file_date (:class:`datetime.datetime`): Datetime when the file was uploaded. """ __slots__ = ( @@ -69,7 +69,7 @@ def __init__( self, file_id: str, file_unique_id: str, - file_date: int, + file_date: datetime, file_size: int, credentials: Optional["FileCredentials"] = None, *, @@ -81,7 +81,7 @@ def __init__( self.file_id: str = file_id self.file_unique_id: str = file_unique_id self.file_size: int = file_size - self.file_date: int = file_date + self.file_date: datetime = file_date # Optionals self._credentials: Optional[FileCredentials] = credentials @@ -111,6 +111,9 @@ def de_json_decrypted( if not data: return None + loc_tzinfo = extract_tzinfo_from_defaults(bot) + + data["file_date"] = from_timestamp(data.get("last_error_date"), tzinfo=loc_tzinfo) data["credentials"] = credentials return super().de_json(data=data, bot=bot) diff --git a/telegram/_payment/orderinfo.py b/telegram/_payment/orderinfo.py index f7d7ac73402..137b6b6b161 100644 --- a/telegram/_payment/orderinfo.py +++ b/telegram/_payment/orderinfo.py @@ -56,7 +56,7 @@ def __init__( name: Optional[str] = None, phone_number: Optional[str] = None, email: Optional[str] = None, - shipping_address: Optional[str] = None, + shipping_address: Optional[ShippingAddress] = None, *, api_kwargs: Optional[JSONDict] = None, ): @@ -64,7 +64,7 @@ def __init__( self.name: Optional[str] = name self.phone_number: Optional[str] = phone_number self.email: Optional[str] = email - self.shipping_address: Optional[str] = shipping_address + self.shipping_address: Optional[ShippingAddress] = shipping_address self._id_attrs = (self.name, self.phone_number, self.email, self.shipping_address) From 4a9adc16cdb7f0c5d92b0b4ec08508b1c6ca20f0 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Mon, 4 Sep 2023 17:28:13 +0400 Subject: [PATCH 06/15] Fix other tests --- telegram/_bot.py | 4 +- telegram/_callbackquery.py | 4 +- telegram/_chat.py | 10 ++--- telegram/_message.py | 4 +- telegram/_payment/shippingquery.py | 4 +- telegram/ext/_extbot.py | 44 +++++++++---------- .../test_passportelementerrorfiles.py | 6 +-- ...st_passportelementerrortranslationfiles.py | 6 +-- 8 files changed, 41 insertions(+), 41 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index f3cff6f13f5..6bf72f1b2dd 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -5540,7 +5540,7 @@ async def add_sticker_to_set( self, user_id: int, name: str, - sticker: Optional[InputSticker], + sticker: Optional["InputSticker"], *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = 20, @@ -5639,7 +5639,7 @@ async def create_new_sticker_set( user_id: int, name: str, title: str, - stickers: Optional[Sequence[InputSticker]], + stickers: Optional[Sequence["InputSticker"]], sticker_format: Optional[str], sticker_type: Optional[str] = None, needs_repainting: Optional[bool] = None, diff --git a/telegram/_callbackquery.py b/telegram/_callbackquery.py index d2c242e3df9..0ecb11f77cb 100644 --- a/telegram/_callbackquery.py +++ b/telegram/_callbackquery.py @@ -531,7 +531,7 @@ async def stop_message_live_location( async def set_game_score( self, - user_id: Union[int, str], + user_id: int, score: int, force: Optional[bool] = None, disable_edit_message: Optional[bool] = None, @@ -589,7 +589,7 @@ async def set_game_score( async def get_game_high_scores( self, - user_id: Union[int, str], + user_id: int, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, diff --git a/telegram/_chat.py b/telegram/_chat.py index a45b295cc89..5dea8ccc7a2 100644 --- a/telegram/_chat.py +++ b/telegram/_chat.py @@ -677,7 +677,7 @@ async def get_member_count( async def get_member( self, - user_id: Union[str, int], + user_id: int, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -707,7 +707,7 @@ async def get_member( async def ban_member( self, - user_id: Union[str, int], + user_id: int, revoke_messages: Optional[bool] = None, until_date: Optional[Union[int, datetime]] = None, *, @@ -877,7 +877,7 @@ async def unban_chat( async def unban_member( self, - user_id: Union[str, int], + user_id: int, only_if_banned: Optional[bool] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, @@ -909,7 +909,7 @@ async def unban_member( async def promote_member( self, - user_id: Union[str, int], + user_id: int, can_change_info: Optional[bool] = None, can_post_messages: Optional[bool] = None, can_edit_messages: Optional[bool] = None, @@ -970,7 +970,7 @@ async def promote_member( async def restrict_member( self, - user_id: Union[str, int], + user_id: int, permissions: ChatPermissions, until_date: Optional[Union[int, datetime]] = None, use_independent_chat_permissions: Optional[bool] = None, diff --git a/telegram/_message.py b/telegram/_message.py index 40ed17f795d..6cfd21fbcda 100644 --- a/telegram/_message.py +++ b/telegram/_message.py @@ -2771,7 +2771,7 @@ async def stop_live_location( async def set_game_score( self, - user_id: Union[int, str], + user_id: int, score: int, force: Optional[bool] = None, disable_edit_message: Optional[bool] = None, @@ -2816,7 +2816,7 @@ async def set_game_score( async def get_game_high_scores( self, - user_id: Union[int, str], + user_id: int, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, diff --git a/telegram/_payment/shippingquery.py b/telegram/_payment/shippingquery.py index 85e94170d9e..5e393ec2ee6 100644 --- a/telegram/_payment/shippingquery.py +++ b/telegram/_payment/shippingquery.py @@ -21,7 +21,6 @@ from typing import TYPE_CHECKING, Optional, Sequence from telegram._payment.shippingaddress import ShippingAddress -from telegram._payment.shippingoption import ShippingOption from telegram._telegramobject import TelegramObject from telegram._user import User from telegram._utils.defaultvalue import DEFAULT_NONE @@ -29,6 +28,7 @@ if TYPE_CHECKING: from telegram import Bot + from telegram._payment.shippingoption import ShippingOption class ShippingQuery(TelegramObject): @@ -92,7 +92,7 @@ def de_json(cls, data: Optional[JSONDict], bot: "Bot") -> Optional["ShippingQuer async def answer( # pylint: disable=invalid-name self, ok: bool, - shipping_options: Optional[Sequence[ShippingOption]] = None, + shipping_options: Optional[Sequence["ShippingOption"]] = None, error_message: Optional[str] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, diff --git a/telegram/ext/_extbot.py b/telegram/ext/_extbot.py index d92146d15cd..5f3cd9654b7 100644 --- a/telegram/ext/_extbot.py +++ b/telegram/ext/_extbot.py @@ -733,9 +733,9 @@ async def get_chat( async def add_sticker_to_set( self, - user_id: Union[str, int], + user_id: int, name: str, - sticker: Optional[InputSticker], + sticker: Optional["InputSticker"], *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = 20, @@ -845,7 +845,7 @@ async def answer_shipping_query( self, shipping_query_id: str, ok: bool, - shipping_options: Optional[Sequence[ShippingOption]] = None, + shipping_options: Optional[Sequence["ShippingOption"]] = None, error_message: Optional[str] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, @@ -914,7 +914,7 @@ async def approve_chat_join_request( async def ban_chat_member( self, chat_id: Union[str, int], - user_id: Union[str, int], + user_id: int, until_date: Optional[Union[int, datetime]] = None, revoke_messages: Optional[bool] = None, *, @@ -1047,10 +1047,10 @@ async def create_invoice_link( async def create_new_sticker_set( self, - user_id: Union[str, int], + user_id: int, name: str, title: str, - stickers: Optional[Sequence[InputSticker]], + stickers: Optional[Sequence["InputSticker"]], sticker_format: Optional[str], sticker_type: Optional[str] = None, needs_repainting: Optional[bool] = None, @@ -1426,7 +1426,7 @@ async def edit_message_reply_markup( chat_id: Optional[Union[str, int]] = None, message_id: Optional[int] = None, inline_message_id: Optional[str] = None, - reply_markup: Optional["InlineKeyboardMarkup"] = None, + reply_markup: Optional[InlineKeyboardMarkup] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -1554,7 +1554,7 @@ async def get_chat_administrators( async def get_chat_member( self, chat_id: Union[str, int], - user_id: Union[str, int], + user_id: int, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -1655,8 +1655,8 @@ async def get_forum_topic_icon_stickers( async def get_game_high_scores( self, - user_id: Union[int, str], - chat_id: Optional[Union[str, int]] = None, + user_id: int, + chat_id: Optional[int] = None, message_id: Optional[int] = None, inline_message_id: Optional[str] = None, *, @@ -1781,7 +1781,7 @@ async def get_custom_emoji_stickers( async def get_user_profile_photos( self, - user_id: Union[str, int], + user_id: int, offset: Optional[int] = None, limit: Optional[int] = None, *, @@ -2032,7 +2032,7 @@ async def pin_chat_message( async def promote_chat_member( self, chat_id: Union[str, int], - user_id: Union[str, int], + user_id: int, can_change_info: Optional[bool] = None, can_post_messages: Optional[bool] = None, can_edit_messages: Optional[bool] = None, @@ -2100,7 +2100,7 @@ async def reopen_forum_topic( async def restrict_chat_member( self, chat_id: Union[str, int], - user_id: Union[str, int], + user_id: int, permissions: ChatPermissions, until_date: Optional[Union[int, datetime]] = None, use_independent_chat_permissions: Optional[bool] = None, @@ -2397,7 +2397,7 @@ async def send_document( async def send_game( self, - chat_id: Union[int, str], + chat_id: int, game_short_name: str, disable_notification: DVInput[bool] = DEFAULT_NONE, reply_to_message_id: Optional[int] = None, @@ -2958,7 +2958,7 @@ async def send_voice( async def set_chat_administrator_custom_title( self, chat_id: Union[int, str], - user_id: Union[int, str], + user_id: int, custom_title: str, *, read_timeout: ODVInput[float] = DEFAULT_NONE, @@ -3115,9 +3115,9 @@ async def set_chat_title( async def set_game_score( self, - user_id: Union[int, str], + user_id: int, score: int, - chat_id: Optional[Union[str, int]] = None, + chat_id: Optional[int] = None, message_id: Optional[int] = None, inline_message_id: Optional[str] = None, force: Optional[bool] = None, @@ -3193,8 +3193,8 @@ async def set_my_default_administrator_rights( async def set_passport_data_errors( self, - user_id: Union[str, int], - errors: Sequence[PassportElementError], + user_id: int, + errors: Sequence["PassportElementError"], *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -3238,7 +3238,7 @@ async def set_sticker_position_in_set( async def set_sticker_set_thumbnail( self, name: str, - user_id: Union[str, int], + user_id: int, thumbnail: Optional[FileInput] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, @@ -3320,7 +3320,7 @@ async def stop_message_live_location( async def unban_chat_member( self, chat_id: Union[str, int], - user_id: Union[str, int], + user_id: int, only_if_banned: Optional[bool] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, @@ -3449,7 +3449,7 @@ async def unpin_all_general_forum_topic_messages( async def upload_sticker_file( self, - user_id: Union[str, int], + user_id: int, sticker: Optional[FileInput], sticker_format: Optional[str], *, diff --git a/tests/_passport/test_passportelementerrorfiles.py b/tests/_passport/test_passportelementerrorfiles.py index 507c222c4e3..07d09f0a6a9 100644 --- a/tests/_passport/test_passportelementerrorfiles.py +++ b/tests/_passport/test_passportelementerrorfiles.py @@ -34,7 +34,7 @@ def passport_element_error_files(): class TestPassportElementErrorFilesBase: source = "files" type_ = "test_type" - file_hashes = ["hash1", "hash2"] + file_hashes = ("hash1", "hash2") message = "Error message" @@ -48,7 +48,7 @@ def test_slot_behaviour(self, passport_element_error_files): def test_expected_values(self, passport_element_error_files): assert passport_element_error_files.source == self.source assert passport_element_error_files.type == self.type_ - assert isinstance(passport_element_error_files.file_hashes, list) + assert isinstance(passport_element_error_files.file_hashes, tuple) assert passport_element_error_files.file_hashes == self.file_hashes assert passport_element_error_files.message == self.message @@ -59,7 +59,7 @@ def test_to_dict(self, passport_element_error_files): assert passport_element_error_files_dict["source"] == passport_element_error_files.source assert passport_element_error_files_dict["type"] == passport_element_error_files.type assert ( - passport_element_error_files_dict["file_hashes"] + tuple(passport_element_error_files_dict["file_hashes"]) == passport_element_error_files.file_hashes ) assert passport_element_error_files_dict["message"] == passport_element_error_files.message diff --git a/tests/_passport/test_passportelementerrortranslationfiles.py b/tests/_passport/test_passportelementerrortranslationfiles.py index 3ae5307f6e8..159714f4da9 100644 --- a/tests/_passport/test_passportelementerrortranslationfiles.py +++ b/tests/_passport/test_passportelementerrortranslationfiles.py @@ -50,8 +50,8 @@ def test_slot_behaviour(self, passport_element_error_translation_files): def test_expected_values(self, passport_element_error_translation_files): assert passport_element_error_translation_files.source == self.source assert passport_element_error_translation_files.type == self.type_ - assert isinstance(passport_element_error_translation_files.file_hashes, list) - assert passport_element_error_translation_files.file_hashes == self.file_hashes + assert isinstance(passport_element_error_translation_files.file_hashes, tuple) + assert passport_element_error_translation_files.file_hashes == tuple(self.file_hashes) assert passport_element_error_translation_files.message == self.message def test_to_dict(self, passport_element_error_translation_files): @@ -69,7 +69,7 @@ def test_to_dict(self, passport_element_error_translation_files): == passport_element_error_translation_files.type ) assert ( - passport_element_error_translation_files_dict["file_hashes"] + tuple(passport_element_error_translation_files_dict["file_hashes"]) == passport_element_error_translation_files.file_hashes ) assert ( From 9a1c2c67074730db3188ef5402beba39f3729608 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Mon, 4 Sep 2023 17:33:19 +0400 Subject: [PATCH 07/15] fix deepsource --- telegram/ext/_extbot.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/telegram/ext/_extbot.py b/telegram/ext/_extbot.py index 5f3cd9654b7..a5dc8309c3c 100644 --- a/telegram/ext/_extbot.py +++ b/telegram/ext/_extbot.py @@ -63,17 +63,14 @@ InlineKeyboardMarkup, InlineQueryResultsButton, InputMedia, - InputSticker, Location, MaskPosition, MenuButton, Message, MessageId, - PassportElementError, PhotoSize, Poll, SentWebAppMessage, - ShippingOption, Sticker, StickerSet, Update, @@ -108,8 +105,11 @@ InputMediaDocument, InputMediaPhoto, InputMediaVideo, + InputSticker, LabeledPrice, MessageEntity, + PassportElementError, + ShippingOption, ) from telegram.ext import BaseRateLimiter, Defaults From ae11a5045575dbf3ba9e274cb8ad3f8e4874c5dc Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Tue, 5 Sep 2023 13:45:25 +0400 Subject: [PATCH 08/15] Review: deprecationwarnings and minor formatting changes to test_official --- telegram/_passport/passportelementerrors.py | 53 ++++++++--- .../test_passportelementerrorfiles.py | 18 ++-- ...st_passportelementerrortranslationfiles.py | 18 ++-- tests/test_official.py | 90 ++++++++++--------- 4 files changed, 111 insertions(+), 68 deletions(-) diff --git a/telegram/_passport/passportelementerrors.py b/telegram/_passport/passportelementerrors.py index a0b5432dc93..4b4b2ac52ce 100644 --- a/telegram/_passport/passportelementerrors.py +++ b/telegram/_passport/passportelementerrors.py @@ -19,11 +19,12 @@ # pylint: disable=redefined-builtin """This module contains the classes that represent Telegram PassportElementError.""" -from typing import Optional, Sequence, Tuple +from typing import List, Optional from telegram._telegramobject import TelegramObject -from telegram._utils.argumentparsing import parse_sequence_arg from telegram._utils.types import JSONDict +from telegram._utils.warnings import warn +from telegram.warnings import PTBDeprecationWarning class PassportElementError(TelegramObject): @@ -167,24 +168,23 @@ class PassportElementErrorFiles(PassportElementError): type (:obj:`str`): The section of the user's Telegram Passport which has the issue, one of ``"utility_bill"``, ``"bank_statement"``, ``"rental_agreement"``, ``"passport_registration"``, ``"temporary_registration"``. - file_hashes (Sequence[:obj:`str`]): List of base64-encoded file hashes. + file_hashes (List[:obj:`str`]): List of base64-encoded file hashes. message (:obj:`str`): Error message. Attributes: type (:obj:`str`): The section of the user's Telegram Passport which has the issue, one of ``"utility_bill"``, ``"bank_statement"``, ``"rental_agreement"``, ``"passport_registration"``, ``"temporary_registration"``. - file_hashes (Tuple[:obj:`str`]): List of base64-encoded file hashes. message (:obj:`str`): Error message. """ - __slots__ = ("file_hashes",) + __slots__ = ("_file_hashes",) def __init__( self, type: str, - file_hashes: Sequence[str], + file_hashes: List[str], message: str, *, api_kwargs: Optional[JSONDict] = None, @@ -192,10 +192,24 @@ def __init__( # Required super().__init__("files", type, message, api_kwargs=api_kwargs) with self._unfrozen(): - self.file_hashes: Tuple[str, ...] = parse_sequence_arg(file_hashes) + self._file_hashes: List[str] = file_hashes self._id_attrs = (self.source, self.type, self.message, *tuple(file_hashes)) + @property + def file_hashes(self) -> List[str]: + """List of base64-encoded file hashes. + + .. deprecated:: NEXT.VERSION + This attribute will return a tuple instead of a list in v22. + """ + warn( + "The attribute `file_hashes` will return a tuple instead of a list in v22.", + PTBDeprecationWarning, + stacklevel=2, + ) + return self._file_hashes + class PassportElementErrorFrontSide(PassportElementError): """ @@ -363,7 +377,7 @@ class PassportElementErrorTranslationFiles(PassportElementError): one of ``"passport"``, ``"driver_license"``, ``"identity_card"``, ``"internal_passport"``, ``"utility_bill"``, ``"bank_statement"``, ``"rental_agreement"``, ``"passport_registration"``, ``"temporary_registration"``. - file_hashes (Sequence[:obj:`str`]): List of base64-encoded file hashes. + file_hashes (List[:obj:`str`]): List of base64-encoded file hashes. message (:obj:`str`): Error message. Attributes: @@ -371,17 +385,16 @@ class PassportElementErrorTranslationFiles(PassportElementError): one of ``"passport"``, ``"driver_license"``, ``"identity_card"``, ``"internal_passport"``, ``"utility_bill"``, ``"bank_statement"``, ``"rental_agreement"``, ``"passport_registration"``, ``"temporary_registration"``. - file_hashes (Tuple[:obj:`str`]): List of base64-encoded file hashes. message (:obj:`str`): Error message. """ - __slots__ = ("file_hashes",) + __slots__ = ("_file_hashes",) def __init__( self, type: str, - file_hashes: Sequence[str], + file_hashes: List[str], message: str, *, api_kwargs: Optional[JSONDict] = None, @@ -389,10 +402,26 @@ def __init__( # Required super().__init__("translation_files", type, message, api_kwargs=api_kwargs) with self._unfrozen(): - self.file_hashes: Tuple[str, ...] = parse_sequence_arg(file_hashes) + self._file_hashes: List[str] = file_hashes self._id_attrs = (self.source, self.type, self.message, *tuple(file_hashes)) + @property + def file_hashes(self) -> List[str]: + """List of base64-encoded file hashes. + + .. deprecated:: NEXT.VERSION + This attribute will return a tuple instead of a list in v22. + """ + warn( + "The attribute `file_hashes` will return a tuple instead of a list in v22. See the " + "stability policy: " + "https://docs.python-telegram-bot.org/en/stable/stability_policy.html", + PTBDeprecationWarning, + stacklevel=2, + ) + return self._file_hashes + class PassportElementErrorUnspecified(PassportElementError): """ diff --git a/tests/_passport/test_passportelementerrorfiles.py b/tests/_passport/test_passportelementerrorfiles.py index 07d09f0a6a9..659a627ea17 100644 --- a/tests/_passport/test_passportelementerrorfiles.py +++ b/tests/_passport/test_passportelementerrorfiles.py @@ -19,6 +19,7 @@ import pytest from telegram import PassportElementErrorFiles, PassportElementErrorSelfie +from telegram.warnings import PTBDeprecationWarning from tests.auxil.slots import mro_slots @@ -34,7 +35,7 @@ def passport_element_error_files(): class TestPassportElementErrorFilesBase: source = "files" type_ = "test_type" - file_hashes = ("hash1", "hash2") + file_hashes = ["hash1", "hash2"] message = "Error message" @@ -48,7 +49,7 @@ def test_slot_behaviour(self, passport_element_error_files): def test_expected_values(self, passport_element_error_files): assert passport_element_error_files.source == self.source assert passport_element_error_files.type == self.type_ - assert isinstance(passport_element_error_files.file_hashes, tuple) + assert isinstance(passport_element_error_files.file_hashes, list) assert passport_element_error_files.file_hashes == self.file_hashes assert passport_element_error_files.message == self.message @@ -58,12 +59,17 @@ def test_to_dict(self, passport_element_error_files): assert isinstance(passport_element_error_files_dict, dict) assert passport_element_error_files_dict["source"] == passport_element_error_files.source assert passport_element_error_files_dict["type"] == passport_element_error_files.type - assert ( - tuple(passport_element_error_files_dict["file_hashes"]) - == passport_element_error_files.file_hashes - ) assert passport_element_error_files_dict["message"] == passport_element_error_files.message + def test_file_hashes_deprecated(self, passport_element_error_files, recwarn): + passport_element_error_files.file_hashes + assert len(recwarn) == 1 + assert "The attribute `file_hashes` will return a tuple instead of a list in v22." in str( + recwarn[0].message + ) + assert recwarn[0].category is PTBDeprecationWarning + assert recwarn[0].filename == __file__ + def test_equality(self): a = PassportElementErrorFiles(self.type_, self.file_hashes, self.message) b = PassportElementErrorFiles(self.type_, self.file_hashes, self.message) diff --git a/tests/_passport/test_passportelementerrortranslationfiles.py b/tests/_passport/test_passportelementerrortranslationfiles.py index 159714f4da9..8023657b35a 100644 --- a/tests/_passport/test_passportelementerrortranslationfiles.py +++ b/tests/_passport/test_passportelementerrortranslationfiles.py @@ -19,6 +19,7 @@ import pytest from telegram import PassportElementErrorSelfie, PassportElementErrorTranslationFiles +from telegram.warnings import PTBDeprecationWarning from tests.auxil.slots import mro_slots @@ -50,8 +51,8 @@ def test_slot_behaviour(self, passport_element_error_translation_files): def test_expected_values(self, passport_element_error_translation_files): assert passport_element_error_translation_files.source == self.source assert passport_element_error_translation_files.type == self.type_ - assert isinstance(passport_element_error_translation_files.file_hashes, tuple) - assert passport_element_error_translation_files.file_hashes == tuple(self.file_hashes) + assert isinstance(passport_element_error_translation_files.file_hashes, list) + assert passport_element_error_translation_files.file_hashes == self.file_hashes assert passport_element_error_translation_files.message == self.message def test_to_dict(self, passport_element_error_translation_files): @@ -68,15 +69,20 @@ def test_to_dict(self, passport_element_error_translation_files): passport_element_error_translation_files_dict["type"] == passport_element_error_translation_files.type ) - assert ( - tuple(passport_element_error_translation_files_dict["file_hashes"]) - == passport_element_error_translation_files.file_hashes - ) assert ( passport_element_error_translation_files_dict["message"] == passport_element_error_translation_files.message ) + def test_file_hashes_deprecated(self, passport_element_error_translation_files, recwarn): + passport_element_error_translation_files.file_hashes + assert len(recwarn) == 1 + assert "The attribute `file_hashes` will return a tuple instead of a list in v22." in str( + recwarn[0].message + ) + assert recwarn[0].category is PTBDeprecationWarning + assert recwarn[0].filename == __file__ + def test_equality(self): a = PassportElementErrorTranslationFiles(self.type_, self.file_hashes, self.message) b = PassportElementErrorTranslationFiles(self.type_, self.file_hashes, self.message) diff --git a/tests/test_official.py b/tests/test_official.py index e8fbd4d72b9..96ffd607e2a 100644 --- a/tests/test_official.py +++ b/tests/test_official.py @@ -30,6 +30,7 @@ import telegram from telegram._utils.defaultvalue import DefaultValue from telegram._utils.types import DVInput, FileInput, ODVInput +from telegram.ext import Defaults from tests.auxil.envvars import env_var_2_bool IGNORED_OBJECTS = ("ResponseParameters", "CallbackGame") @@ -64,6 +65,28 @@ "InputFile": {"attach", "filename", "obj"}, } +# Types for certain parameters accepted by PTB but not in the official API +ADDITIONAL_TYPES = { + "photo": ForwardRef("PhotoSize"), + "video": ForwardRef("Video"), + "video_note": ForwardRef("VideoNote"), + "audio": ForwardRef("Audio"), + "document": ForwardRef("Document"), + "animation": ForwardRef("Animation"), + "voice": ForwardRef("Voice"), + "sticker": ForwardRef("Sticker"), +} + +# Exceptions to the "Array of" types, where we accept more types than the official API +# key: parameter name, value: type which must be present in the annotation +ARRAY_OF_EXCEPTIONS = { + "results": "InlineQueryResult", # + Callable + "commands": "BotCommand", # + tuple[str, str] + "keyboard": "KeyboardButton", # + sequence[sequence[str]] + # TODO: Deprecated and will be corrected (and removed) in next major PTB version: + "file_hashes": "list[str]", +} + def _get_params_base(object_name: str, search_dict: dict[str, set[Any]]) -> set[Any]: """Helper function for the *_params functions below. @@ -316,8 +339,12 @@ def check_param_type( tg_param_type: str = tg_parameter[1] # Type of parameter as specified in the docs is_class = inspect.isclass(obj) + # Let's check for a match: mapped: set[type] = _get_params_base(tg_param_type, TYPE_MAPPING) + # We should have a maximum of one match. + assert len(mapped) <= 1, f"More than one match found for {tg_param_type}" + if not mapped: # no match found, it's from telegram module # it could be a list of objects, so let's check that: objs = _extract_words(tg_param_type) @@ -329,7 +356,7 @@ def check_param_type( mapped_type = ( getattr(telegram, tg_param_type), # This will fail if it's not from telegram mod ForwardRef(tg_param_type), - tg_param_type, # for some reason, some annotations are not ForwardRefs, i.e. str. + tg_param_type, # for some reason, some annotations are just a string. ) print("len mapped 0 ", mapped_type) elif len(mapped) == 1: @@ -346,8 +373,8 @@ def check_param_type( # Some cleaning: # Remove 'Optional[...]' from the annotation if it's present. We do it this way since: 1) # we already check if argument should be optional or not + type checkers will complain. - # 2) we don't want to deal with Optional[..] skewing the equality check of our annotation - # with the official API, i.e. Optional[int] == int. + # 2) we want to check if our `obj` is same as API's `obj`, and since python evaluates + # `Optional[obj] != obj` we have to remove the Optional, so that we can compare the two. if type(None) in ptb_annotation: print(f"found None in {ptb_param.name}") ptb_annotation.remove(type(None)) @@ -372,16 +399,11 @@ def check_param_type( if "Array of " in tg_param_type: print("array of ", ptb_param.annotation) + assert mapped_type is Sequence # For exceptions just check if they contain the annotation - array_of_exceptions = { # We accept more types than the official API - "results": "InlineQueryResult", - "commands": "BotCommand", - "keyboard": "KeyboardButton", - } - if ptb_param.name in array_of_exceptions: - return array_of_exceptions[ptb_param.name] in str(ptb_annotation) - - # let's import obj + if ptb_param.name in ARRAY_OF_EXCEPTIONS: + return ARRAY_OF_EXCEPTIONS[ptb_param.name] in str(ptb_annotation) + pattern = r"Array of(?: Array of)? ([\w\,\s]*)" obj_match: re.Match | None = re.search(pattern, tg_param_type) # extract obj from string if obj_match is None: @@ -400,7 +422,6 @@ def check_param_type( print("from TYPE_MAPPING") unionized_objs = [array_of_mapped.pop()] - assert mapped_type is Sequence # This means it is Array of Array of [obj] if "Array of Array of" in tg_param_type: # print('isinstance 1', Sequence[Sequence[obj]], ptb_annotation) @@ -410,44 +431,25 @@ def check_param_type( # print('isinstance 2', mapped_type[obj], ptb_annotation) return any(mapped_type[o] == ptb_annotation for o in unionized_objs) - DEFAULT_VAL_PARAMS = { - "parse_mode", - "disable_notification", - "allow_sending_without_reply", - "protect_content", - "disable_web_page_preview", - "explanation_parse_mode", - } - # Special case for when the parameter is a default value parameter - if ptb_param.name in DEFAULT_VAL_PARAMS: - # Check if it's DVInput or ODVInput - for param_type in [DVInput, ODVInput]: - parsed = param_type[mapped_type] - if ptb_annotation == parsed: - return True - # print(f"cause {mapped_type} != {ptb_annotation} for {ptb_param.name}") - return False + for name, _ in inspect.getmembers(Defaults, lambda x: isinstance(x, property)): + if name in ptb_param.name: # no strict == since we have a param: `explanation_parse_mode` + # Check if it's DVInput or ODVInput + for param_type in [DVInput, ODVInput]: + parsed = param_type[mapped_type] + if ptb_annotation == parsed: + return True + # print(f"cause {mapped_type} != {ptb_annotation} for {ptb_param.name}") + return False # Special case for send_* methods where we accept more types than the official API: - additional_types = { - "photo": ForwardRef("PhotoSize"), - "video": ForwardRef("Video"), - "video_note": ForwardRef("VideoNote"), - "audio": ForwardRef("Audio"), - "document": ForwardRef("Document"), - "animation": ForwardRef("Animation"), - "voice": ForwardRef("Voice"), - "sticker": ForwardRef("Sticker"), - } - if ( - ptb_param.name in additional_types + ptb_param.name in ADDITIONAL_TYPES and not isinstance(mapped_type, tuple) and obj.__name__.startswith("send") ): print("additional_types ") - mapped_type = mapped_type | additional_types[ptb_param.name] + mapped_type = mapped_type | ADDITIONAL_TYPES[ptb_param.name] # Special cases for other methods that accept more types than the official API, and are # too complex to compare/predict with official API: @@ -466,7 +468,7 @@ def check_param_type( ptb_annotation = exception_type # Special case for datetimes - if re.search(r"([_]+|\b)date[^\w]*\b", ptb_param.name): + if re.search(r"([_]+|\b)date[^\w]*\b", ptb_param.name) or "Unix time" in tg_parameter[-1]: print("datetime found") # If it's a class, we only accept datetime as the parameter mapped_type = datetime if is_class else mapped_type | datetime From 3030b3c29a1bae5b5f9308b70d79ef67f6660d86 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Tue, 5 Sep 2023 14:13:24 +0400 Subject: [PATCH 09/15] Review: deprecation for file_date & override to_dict Also rearrange 2 tests --- telegram/_passport/passportelementerrors.py | 10 +++++ telegram/_passport/passportfile.py | 40 ++++++++++++++----- .../test_passportelementerrorfiles.py | 20 ++++++---- ...st_passportelementerrortranslationfiles.py | 20 ++++++---- tests/_passport/test_passportfile.py | 11 +++++ 5 files changed, 76 insertions(+), 25 deletions(-) diff --git a/telegram/_passport/passportelementerrors.py b/telegram/_passport/passportelementerrors.py index 4b4b2ac52ce..95a43fb9966 100644 --- a/telegram/_passport/passportelementerrors.py +++ b/telegram/_passport/passportelementerrors.py @@ -196,6 +196,11 @@ def __init__( self._id_attrs = (self.source, self.type, self.message, *tuple(file_hashes)) + def to_dict(self, recursive: bool = True) -> JSONDict: + data = super().to_dict(recursive) + data["file_hashes"] = self._file_hashes + return data + @property def file_hashes(self) -> List[str]: """List of base64-encoded file hashes. @@ -406,6 +411,11 @@ def __init__( self._id_attrs = (self.source, self.type, self.message, *tuple(file_hashes)) + def to_dict(self, recursive: bool = True) -> JSONDict: + data = super().to_dict(recursive) + data["file_hashes"] = self._file_hashes + return data + @property def file_hashes(self) -> List[str]: """List of base64-encoded file hashes. diff --git a/telegram/_passport/passportfile.py b/telegram/_passport/passportfile.py index fa6b094c2d3..f12ef736362 100644 --- a/telegram/_passport/passportfile.py +++ b/telegram/_passport/passportfile.py @@ -18,13 +18,13 @@ # along with this program. If not, see [http://www.gnu.org/licenses/]. """This module contains an object that represents a Encrypted PassportFile.""" -from datetime import datetime from typing import TYPE_CHECKING, List, Optional, Tuple from telegram._telegramobject import TelegramObject -from telegram._utils.datetime import extract_tzinfo_from_defaults, from_timestamp from telegram._utils.defaultvalue import DEFAULT_NONE from telegram._utils.types import JSONDict, ODVInput +from telegram._utils.warnings import warn +from telegram.warnings import PTBDeprecationWarning if TYPE_CHECKING: from telegram import Bot, File, FileCredentials @@ -45,7 +45,10 @@ class PassportFile(TelegramObject): is supposed to be the same over time and for different bots. Can't be used to download or reuse the file. file_size (:obj:`int`): File size in bytes. - file_date (:class:`datetime.datetime`): Datetime when the file was uploaded. + file_date (:obj:`int`): Unix time when the file was uploaded. + + .. deprecated:: NEXT.VERSION + This argument will only accept a datetime instead of an integer in v22. Attributes: file_id (:obj:`str`): Identifier for this file, which can be used to download @@ -54,11 +57,10 @@ class PassportFile(TelegramObject): is supposed to be the same over time and for different bots. Can't be used to download or reuse the file. file_size (:obj:`int`): File size in bytes. - file_date (:class:`datetime.datetime`): Datetime when the file was uploaded. """ __slots__ = ( - "file_date", + "_file_date", "file_id", "file_size", "_credentials", @@ -69,7 +71,7 @@ def __init__( self, file_id: str, file_unique_id: str, - file_date: datetime, + file_date: int, file_size: int, credentials: Optional["FileCredentials"] = None, *, @@ -81,7 +83,7 @@ def __init__( self.file_id: str = file_id self.file_unique_id: str = file_unique_id self.file_size: int = file_size - self.file_date: datetime = file_date + self._file_date: int = file_date # Optionals self._credentials: Optional[FileCredentials] = credentials @@ -90,6 +92,25 @@ def __init__( self._freeze() + def to_dict(self, recursive: bool = True) -> JSONDict: + data = super().to_dict(recursive) + data["file_date"] = self._file_date + return data + + @property + def file_date(self) -> int: + """:obj:`int`: Unix time when the file was uploaded. + + .. deprecated:: NEXT.VERSION + This attribute will return a datetime instead of a integer in v22. + """ + warn( + "The attribute `file_date` will return a datetime instead of an integer in v22.", + PTBDeprecationWarning, + stacklevel=2, + ) + return self._file_date + @classmethod def de_json_decrypted( cls, data: Optional[JSONDict], bot: "Bot", credentials: "FileCredentials" @@ -111,9 +132,10 @@ def de_json_decrypted( if not data: return None - loc_tzinfo = extract_tzinfo_from_defaults(bot) + # TODO: Uncomment for v22 + # loc_tzinfo = extract_tzinfo_from_defaults(bot) - data["file_date"] = from_timestamp(data.get("last_error_date"), tzinfo=loc_tzinfo) + # data["file_date"] = from_timestamp(data.get("file_date"), tzinfo=loc_tzinfo) data["credentials"] = credentials return super().de_json(data=data, bot=bot) diff --git a/tests/_passport/test_passportelementerrorfiles.py b/tests/_passport/test_passportelementerrorfiles.py index 659a627ea17..121befc33f2 100644 --- a/tests/_passport/test_passportelementerrorfiles.py +++ b/tests/_passport/test_passportelementerrorfiles.py @@ -60,15 +60,10 @@ def test_to_dict(self, passport_element_error_files): assert passport_element_error_files_dict["source"] == passport_element_error_files.source assert passport_element_error_files_dict["type"] == passport_element_error_files.type assert passport_element_error_files_dict["message"] == passport_element_error_files.message - - def test_file_hashes_deprecated(self, passport_element_error_files, recwarn): - passport_element_error_files.file_hashes - assert len(recwarn) == 1 - assert "The attribute `file_hashes` will return a tuple instead of a list in v22." in str( - recwarn[0].message + assert ( + passport_element_error_files_dict["file_hashes"] + == passport_element_error_files.file_hashes ) - assert recwarn[0].category is PTBDeprecationWarning - assert recwarn[0].filename == __file__ def test_equality(self): a = PassportElementErrorFiles(self.type_, self.file_hashes, self.message) @@ -93,3 +88,12 @@ def test_equality(self): assert a != f assert hash(a) != hash(f) + + def test_file_hashes_deprecated(self, passport_element_error_files, recwarn): + passport_element_error_files.file_hashes + assert len(recwarn) == 1 + assert "The attribute `file_hashes` will return a tuple instead of a list in v22." in str( + recwarn[0].message + ) + assert recwarn[0].category is PTBDeprecationWarning + assert recwarn[0].filename == __file__ diff --git a/tests/_passport/test_passportelementerrortranslationfiles.py b/tests/_passport/test_passportelementerrortranslationfiles.py index 8023657b35a..e43f6ee4c01 100644 --- a/tests/_passport/test_passportelementerrortranslationfiles.py +++ b/tests/_passport/test_passportelementerrortranslationfiles.py @@ -73,15 +73,10 @@ def test_to_dict(self, passport_element_error_translation_files): passport_element_error_translation_files_dict["message"] == passport_element_error_translation_files.message ) - - def test_file_hashes_deprecated(self, passport_element_error_translation_files, recwarn): - passport_element_error_translation_files.file_hashes - assert len(recwarn) == 1 - assert "The attribute `file_hashes` will return a tuple instead of a list in v22." in str( - recwarn[0].message + assert ( + passport_element_error_translation_files_dict["file_hashes"] + == passport_element_error_translation_files.file_hashes ) - assert recwarn[0].category is PTBDeprecationWarning - assert recwarn[0].filename == __file__ def test_equality(self): a = PassportElementErrorTranslationFiles(self.type_, self.file_hashes, self.message) @@ -106,3 +101,12 @@ def test_equality(self): assert a != f assert hash(a) != hash(f) + + def test_file_hashes_deprecated(self, passport_element_error_translation_files, recwarn): + passport_element_error_translation_files.file_hashes + assert len(recwarn) == 1 + assert "The attribute `file_hashes` will return a tuple instead of a list in v22." in str( + recwarn[0].message + ) + assert recwarn[0].category is PTBDeprecationWarning + assert recwarn[0].filename == __file__ diff --git a/tests/_passport/test_passportfile.py b/tests/_passport/test_passportfile.py index 2492ba66afd..37158a4af42 100644 --- a/tests/_passport/test_passportfile.py +++ b/tests/_passport/test_passportfile.py @@ -19,6 +19,7 @@ import pytest from telegram import Bot, File, PassportElementError, PassportFile +from telegram.warnings import PTBDeprecationWarning from tests.auxil.bot_method_checks import ( check_defaults_handling, check_shortcut_call, @@ -88,6 +89,16 @@ def test_equality(self): assert a != e assert hash(a) != hash(e) + def test_file_date_deprecated(self, passport_file, recwarn): + passport_file.file_date + assert len(recwarn) == 1 + assert ( + "The attribute `file_date` will return a datetime instead of an integer in v22." + in str(recwarn[0].message) + ) + assert recwarn[0].category is PTBDeprecationWarning + assert recwarn[0].filename == __file__ + async def test_get_file_instance_method(self, monkeypatch, passport_file): async def make_assertion(*_, **kwargs): result = kwargs["file_id"] == passport_file.file_id From 9032620529821b11d702f99037b5c5c083af602a Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Tue, 5 Sep 2023 14:33:44 +0400 Subject: [PATCH 10/15] oops I broke few things, so here's the fix Also don't collect test_official on py < 3.10 --- telegram/_passport/passportelementerrors.py | 2 ++ telegram/_passport/passportfile.py | 5 +--- tests/conftest.py | 7 +++++ tests/test_official.py | 33 ++++++++++++--------- 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/telegram/_passport/passportelementerrors.py b/telegram/_passport/passportelementerrors.py index 95a43fb9966..0dff54ceb60 100644 --- a/telegram/_passport/passportelementerrors.py +++ b/telegram/_passport/passportelementerrors.py @@ -197,6 +197,7 @@ def __init__( self._id_attrs = (self.source, self.type, self.message, *tuple(file_hashes)) def to_dict(self, recursive: bool = True) -> JSONDict: + """See :meth:`telegram.TelegramObject.to_dict` for details.""" data = super().to_dict(recursive) data["file_hashes"] = self._file_hashes return data @@ -412,6 +413,7 @@ def __init__( self._id_attrs = (self.source, self.type, self.message, *tuple(file_hashes)) def to_dict(self, recursive: bool = True) -> JSONDict: + """See :meth:`telegram.TelegramObject.to_dict` for details.""" data = super().to_dict(recursive) data["file_hashes"] = self._file_hashes return data diff --git a/telegram/_passport/passportfile.py b/telegram/_passport/passportfile.py index f12ef736362..0cb00f965e1 100644 --- a/telegram/_passport/passportfile.py +++ b/telegram/_passport/passportfile.py @@ -93,6 +93,7 @@ def __init__( self._freeze() def to_dict(self, recursive: bool = True) -> JSONDict: + """See :meth:`telegram.TelegramObject.to_dict` for details.""" data = super().to_dict(recursive) data["file_date"] = self._file_date return data @@ -132,10 +133,6 @@ def de_json_decrypted( if not data: return None - # TODO: Uncomment for v22 - # loc_tzinfo = extract_tzinfo_from_defaults(bot) - - # data["file_date"] = from_timestamp(data.get("file_date"), tzinfo=loc_tzinfo) data["credentials"] = credentials return super().de_json(data=data, bot=bot) diff --git a/tests/conftest.py b/tests/conftest.py index 7c6a9a66173..71ee4da6adc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -50,6 +50,13 @@ import pytz +# Don't collect `test_official.py` on Python 3.10- since it uses newer features like X | Y syntax. +# Docs: https://docs.pytest.org/en/7.1.x/example/pythoncollection.html#customizing-test-collection +collect_ignore = [] +if sys.version_info < (3, 10): + collect_ignore.append("test_official.py") + + # This is here instead of in setup.cfg due to https://github.com/pytest-dev/pytest/issues/8343 def pytest_runtestloop(session: pytest.Session): session.add_marker( diff --git a/tests/test_official.py b/tests/test_official.py index 96ffd607e2a..9226b17dbc0 100644 --- a/tests/test_official.py +++ b/tests/test_official.py @@ -87,6 +87,18 @@ "file_hashes": "list[str]", } +# Special cases for other parameters that accept more types than the official API, and are +# too complex to compare/predict with official API: +EXCEPTIONS = { # (param_name, is_class): reduced form of annotation + ("correct_option_id", False): int, # actual: Literal + ("file_id", False): str, # actual: Union[str, objs_with_file_id_attr] + ("invite_link", False): str, # actual: Union[str, ChatInviteLink] + ("provider_data", False): str, # actual: Union[str, obj] + ("callback_data", True): str, # actual: Union[str, obj] + ("media", True): str, # actual: Union[str, InputMedia*, FileInput] + ("data", True): str, # actual: Union[IdDocumentData, PersonalDetails, ResidentialAddress] +} + def _get_params_base(object_name: str, search_dict: dict[str, set[Any]]) -> set[Any]: """Helper function for the *_params functions below. @@ -451,25 +463,18 @@ def check_param_type( print("additional_types ") mapped_type = mapped_type | ADDITIONAL_TYPES[ptb_param.name] - # Special cases for other methods that accept more types than the official API, and are - # too complex to compare/predict with official API: - exceptions = { # (param_name, is_class): reduced form of annotation - ("correct_option_id", False): int, # actual: Literal - ("file_id", False): str, # actual: Union[str, objs_with_file_id_attr] - ("invite_link", False): str, # actual: Union[str, ChatInviteLink] - ("provider_data", False): str, # actual: Union[str, obj] - ("callback_data", True): str, # actual: Union[str, obj] - ("media", True): str, # actual: Union[str, InputMedia*, FileInput] - ("data", True): str, # actual: Union[IdDocumentData, PersonalDetails, ResidentialAddress] - } - - for (param_name, expected_class), exception_type in exceptions.items(): + for (param_name, expected_class), exception_type in EXCEPTIONS.items(): if ptb_param.name == param_name and is_class is expected_class: ptb_annotation = exception_type # Special case for datetimes if re.search(r"([_]+|\b)date[^\w]*\b", ptb_param.name) or "Unix time" in tg_parameter[-1]: - print("datetime found") + # TODO: Remove this in v22 when it becomes a datetime + datetime_exceptions = { + "file_date", + } + if ptb_param.name in datetime_exceptions: + return True # If it's a class, we only accept datetime as the parameter mapped_type = datetime if is_class else mapped_type | datetime From 2ba821f79e88dd140b082b8b7915cc5f33f61288 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Sun, 10 Sep 2023 00:35:38 +0400 Subject: [PATCH 11/15] Review: v22 -> future major versions --- telegram/_passport/passportelementerrors.py | 13 +++++++------ telegram/_passport/passportfile.py | 8 +++++--- tests/_passport/test_passportelementerrorfiles.py | 5 +++-- .../test_passportelementerrortranslationfiles.py | 5 +++-- tests/_passport/test_passportfile.py | 4 ++-- 5 files changed, 20 insertions(+), 15 deletions(-) diff --git a/telegram/_passport/passportelementerrors.py b/telegram/_passport/passportelementerrors.py index 0dff54ceb60..390380c145d 100644 --- a/telegram/_passport/passportelementerrors.py +++ b/telegram/_passport/passportelementerrors.py @@ -207,10 +207,11 @@ def file_hashes(self) -> List[str]: """List of base64-encoded file hashes. .. deprecated:: NEXT.VERSION - This attribute will return a tuple instead of a list in v22. + This attribute will return a tuple instead of a list in future major versions. """ warn( - "The attribute `file_hashes` will return a tuple instead of a list in v22.", + "The attribute `file_hashes` will return a tuple instead of a list in future major" + " versions.", PTBDeprecationWarning, stacklevel=2, ) @@ -423,12 +424,12 @@ def file_hashes(self) -> List[str]: """List of base64-encoded file hashes. .. deprecated:: NEXT.VERSION - This attribute will return a tuple instead of a list in v22. + This attribute will return a tuple instead of a list in future major versions. """ warn( - "The attribute `file_hashes` will return a tuple instead of a list in v22. See the " - "stability policy: " - "https://docs.python-telegram-bot.org/en/stable/stability_policy.html", + "The attribute `file_hashes` will return a tuple instead of a list in future major" + " versions. See the stability policy:" + " https://docs.python-telegram-bot.org/en/stable/stability_policy.html", PTBDeprecationWarning, stacklevel=2, ) diff --git a/telegram/_passport/passportfile.py b/telegram/_passport/passportfile.py index 0cb00f965e1..dd2a290fe5f 100644 --- a/telegram/_passport/passportfile.py +++ b/telegram/_passport/passportfile.py @@ -48,7 +48,8 @@ class PassportFile(TelegramObject): file_date (:obj:`int`): Unix time when the file was uploaded. .. deprecated:: NEXT.VERSION - This argument will only accept a datetime instead of an integer in v22. + This argument will only accept a datetime instead of an integer in future + major versions. Attributes: file_id (:obj:`str`): Identifier for this file, which can be used to download @@ -103,10 +104,11 @@ def file_date(self) -> int: """:obj:`int`: Unix time when the file was uploaded. .. deprecated:: NEXT.VERSION - This attribute will return a datetime instead of a integer in v22. + This attribute will return a datetime instead of a integer in future major versions. """ warn( - "The attribute `file_date` will return a datetime instead of an integer in v22.", + "The attribute `file_date` will return a datetime instead of an integer in future" + " major versions.", PTBDeprecationWarning, stacklevel=2, ) diff --git a/tests/_passport/test_passportelementerrorfiles.py b/tests/_passport/test_passportelementerrorfiles.py index 121befc33f2..73737516f1d 100644 --- a/tests/_passport/test_passportelementerrorfiles.py +++ b/tests/_passport/test_passportelementerrorfiles.py @@ -92,8 +92,9 @@ def test_equality(self): def test_file_hashes_deprecated(self, passport_element_error_files, recwarn): passport_element_error_files.file_hashes assert len(recwarn) == 1 - assert "The attribute `file_hashes` will return a tuple instead of a list in v22." in str( - recwarn[0].message + assert ( + "The attribute `file_hashes` will return a tuple instead of a list in future major" + " versions." in str(recwarn[0].message) ) assert recwarn[0].category is PTBDeprecationWarning assert recwarn[0].filename == __file__ diff --git a/tests/_passport/test_passportelementerrortranslationfiles.py b/tests/_passport/test_passportelementerrortranslationfiles.py index e43f6ee4c01..58196e713fc 100644 --- a/tests/_passport/test_passportelementerrortranslationfiles.py +++ b/tests/_passport/test_passportelementerrortranslationfiles.py @@ -105,8 +105,9 @@ def test_equality(self): def test_file_hashes_deprecated(self, passport_element_error_translation_files, recwarn): passport_element_error_translation_files.file_hashes assert len(recwarn) == 1 - assert "The attribute `file_hashes` will return a tuple instead of a list in v22." in str( - recwarn[0].message + assert ( + "The attribute `file_hashes` will return a tuple instead of a list in future major" + " versions." in str(recwarn[0].message) ) assert recwarn[0].category is PTBDeprecationWarning assert recwarn[0].filename == __file__ diff --git a/tests/_passport/test_passportfile.py b/tests/_passport/test_passportfile.py index 37158a4af42..7ec9fc41b7b 100644 --- a/tests/_passport/test_passportfile.py +++ b/tests/_passport/test_passportfile.py @@ -93,8 +93,8 @@ def test_file_date_deprecated(self, passport_file, recwarn): passport_file.file_date assert len(recwarn) == 1 assert ( - "The attribute `file_date` will return a datetime instead of an integer in v22." - in str(recwarn[0].message) + "The attribute `file_date` will return a datetime instead of an integer in future" + " major versions." in str(recwarn[0].message) ) assert recwarn[0].category is PTBDeprecationWarning assert recwarn[0].filename == __file__ From 5ac6711fa7707cc62c60c11befadf87b066d6178 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Sun, 10 Sep 2023 01:10:12 +0400 Subject: [PATCH 12/15] Review: some refactoring + warning --- tests/README.rst | 2 +- tests/auxil/envvars.py | 1 + tests/conftest.py | 5 ++++- tests/test_official.py | 9 ++++----- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/README.rst b/tests/README.rst index 0e0808a5c1b..821c8ea3179 100644 --- a/tests/README.rst +++ b/tests/README.rst @@ -72,7 +72,7 @@ complete and correct. To run it, export an environment variable first: $ export TEST_OFFICIAL=true -and then run ``pytest tests/test_official.py``. Note: You need py 3.11+ to run this test. +and then run ``pytest tests/test_official.py``. Note: You need py 3.10+ to run this test. We also have another marker, ``@pytest.mark.dev``, which you can add to tests that you want to run selectively. Use as follows: diff --git a/tests/auxil/envvars.py b/tests/auxil/envvars.py index e7456204341..5a3de55ec80 100644 --- a/tests/auxil/envvars.py +++ b/tests/auxil/envvars.py @@ -29,3 +29,4 @@ def env_var_2_bool(env_var: object) -> bool: GITHUB_ACTION = os.getenv("GITHUB_ACTION", "") TEST_WITH_OPT_DEPS = env_var_2_bool(os.getenv("TEST_WITH_OPT_DEPS", "true")) +RUN_TEST_OFFICIAL = env_var_2_bool(os.getenv("TEST_OFFICIAL")) diff --git a/tests/conftest.py b/tests/conftest.py index 71ee4da6adc..2f96124c2e3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,6 +18,7 @@ # along with this program. If not, see [http://www.gnu.org/licenses/]. import asyncio import datetime +import logging import sys from typing import Dict, List from uuid import uuid4 @@ -40,7 +41,7 @@ from tests.auxil.build_messages import DATE from tests.auxil.ci_bots import BOT_INFO_PROVIDER from tests.auxil.constants import PRIVATE_KEY -from tests.auxil.envvars import TEST_WITH_OPT_DEPS +from tests.auxil.envvars import RUN_TEST_OFFICIAL, TEST_WITH_OPT_DEPS from tests.auxil.files import data_file from tests.auxil.networking import NonchalantHttpxRequest from tests.auxil.pytest_classes import PytestApplication, PytestBot, make_bot @@ -54,6 +55,8 @@ # Docs: https://docs.pytest.org/en/7.1.x/example/pythoncollection.html#customizing-test-collection collect_ignore = [] if sys.version_info < (3, 10): + if RUN_TEST_OFFICIAL: + logging.warning("Skipping test_official.py since it requires Python 3.10+") collect_ignore.append("test_official.py") diff --git a/tests/test_official.py b/tests/test_official.py index 9226b17dbc0..b39987cea38 100644 --- a/tests/test_official.py +++ b/tests/test_official.py @@ -17,7 +17,6 @@ # You should have received a copy of the GNU Lesser Public License # along with this program. If not, see [http://www.gnu.org/licenses/]. import inspect -import os import re from datetime import datetime from types import FunctionType @@ -31,7 +30,7 @@ from telegram._utils.defaultvalue import DefaultValue from telegram._utils.types import DVInput, FileInput, ODVInput from telegram.ext import Defaults -from tests.auxil.envvars import env_var_2_bool +from tests.auxil.envvars import RUN_TEST_OFFICIAL IGNORED_OBJECTS = ("ResponseParameters", "CallbackGame") GLOBALLY_IGNORED_PARAMETERS = { @@ -345,6 +344,7 @@ def check_param_type( "String": {str}, r"Boolean|True": {bool}, r"Float(?: number)?": {float}, + # Distinguishing 1D and 2D Sequences and finding the inner type is done later. r"Array of (?:Array of )?[\w\,\s]*": {Sequence}, r"InputFile(?: or String)?": {FileInput}, } @@ -507,11 +507,10 @@ def _unionizer(annotation: Sequence[Any] | set[Any], forward_ref: bool) -> Any: return union -to_run = env_var_2_bool(os.getenv("TEST_OFFICIAL")) argvalues: list[tuple[Callable[[Tag], None], Tag]] = [] names: list[str] = [] -if to_run: +if RUN_TEST_OFFICIAL: argvalues = [] names = [] request = httpx.get("https://core.telegram.org/bots/api") @@ -534,7 +533,7 @@ def _unionizer(annotation: Sequence[Any] | set[Any], forward_ref: bool) -> Any: names.append(h4.text) -@pytest.mark.skipif(not to_run, reason="test_official is not enabled") +@pytest.mark.skipif(not RUN_TEST_OFFICIAL, reason="test_official is not enabled") @pytest.mark.parametrize(("method", "data"), argvalues=argvalues, ids=names) def test_official(method, data): method(data) From 0bb6b8b7e6874c9c4260ed50f4816d2adaf58bc1 Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Fri, 15 Sep 2023 20:10:34 +0400 Subject: [PATCH 13/15] Review: use re.VERBOSE, make InlineKeyboardMarkup forwardref again also remove print statements --- telegram/_bot.py | 20 ++++++++++---------- telegram/ext/_extbot.py | 18 +++++++++--------- tests/test_official.py | 28 +++++++++++----------------- 3 files changed, 30 insertions(+), 36 deletions(-) diff --git a/telegram/_bot.py b/telegram/_bot.py index 6bf72f1b2dd..64459e84664 100644 --- a/telegram/_bot.py +++ b/telegram/_bot.py @@ -78,7 +78,6 @@ from telegram._files.voice import Voice from telegram._forumtopic import ForumTopic from telegram._games.gamehighscore import GameHighScore -from telegram._inline.inlinekeyboardmarkup import InlineKeyboardMarkup from telegram._inline.inlinequeryresultsbutton import InlineQueryResultsButton from telegram._menubutton import MenuButton from telegram._message import Message @@ -112,6 +111,7 @@ if TYPE_CHECKING: from telegram import ( + InlineKeyboardMarkup, InlineQueryResult, InputFile, InputMediaAudio, @@ -2154,7 +2154,7 @@ async def edit_message_live_location( inline_message_id: Optional[str] = None, latitude: Optional[float] = None, longitude: Optional[float] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, horizontal_accuracy: Optional[float] = None, heading: Optional[int] = None, proximity_alert_radius: Optional[int] = None, @@ -2247,7 +2247,7 @@ async def stop_message_live_location( chat_id: Optional[Union[str, int]] = None, message_id: Optional[int] = None, inline_message_id: Optional[str] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -2525,7 +2525,7 @@ async def send_game( game_short_name: str, disable_notification: DVInput[bool] = DEFAULT_NONE, reply_to_message_id: Optional[int] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, allow_sending_without_reply: ODVInput[bool] = DEFAULT_NONE, protect_content: ODVInput[bool] = DEFAULT_NONE, message_thread_id: Optional[int] = None, @@ -3203,7 +3203,7 @@ async def edit_message_text( inline_message_id: Optional[str] = None, parse_mode: ODVInput[str] = DEFAULT_NONE, disable_web_page_preview: ODVInput[bool] = DEFAULT_NONE, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, entities: Optional[Sequence["MessageEntity"]] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, @@ -3279,7 +3279,7 @@ async def edit_message_caption( message_id: Optional[int] = None, inline_message_id: Optional[str] = None, caption: Optional[str] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, parse_mode: ODVInput[str] = DEFAULT_NONE, caption_entities: Optional[Sequence["MessageEntity"]] = None, *, @@ -3349,7 +3349,7 @@ async def edit_message_media( chat_id: Optional[Union[str, int]] = None, message_id: Optional[int] = None, inline_message_id: Optional[str] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -3412,7 +3412,7 @@ async def edit_message_reply_markup( chat_id: Optional[Union[str, int]] = None, message_id: Optional[int] = None, inline_message_id: Optional[str] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -4156,7 +4156,7 @@ async def send_invoice( is_flexible: Optional[bool] = None, disable_notification: DVInput[bool] = DEFAULT_NONE, reply_to_message_id: Optional[int] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, provider_data: Optional[Union[str, object]] = None, send_phone_number_to_provider: Optional[bool] = None, send_email_to_provider: Optional[bool] = None, @@ -6262,7 +6262,7 @@ async def stop_poll( self, chat_id: Union[int, str], message_id: int, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, diff --git a/telegram/ext/_extbot.py b/telegram/ext/_extbot.py index a5dc8309c3c..ab0edea1ea8 100644 --- a/telegram/ext/_extbot.py +++ b/telegram/ext/_extbot.py @@ -645,7 +645,7 @@ async def stop_poll( self, chat_id: Union[int, str], message_id: int, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -1329,7 +1329,7 @@ async def edit_message_caption( message_id: Optional[int] = None, inline_message_id: Optional[str] = None, caption: Optional[str] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, parse_mode: ODVInput[str] = DEFAULT_NONE, caption_entities: Optional[Sequence["MessageEntity"]] = None, *, @@ -1362,7 +1362,7 @@ async def edit_message_live_location( inline_message_id: Optional[str] = None, latitude: Optional[float] = None, longitude: Optional[float] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, horizontal_accuracy: Optional[float] = None, heading: Optional[int] = None, proximity_alert_radius: Optional[int] = None, @@ -1399,7 +1399,7 @@ async def edit_message_media( chat_id: Optional[Union[str, int]] = None, message_id: Optional[int] = None, inline_message_id: Optional[str] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -1426,7 +1426,7 @@ async def edit_message_reply_markup( chat_id: Optional[Union[str, int]] = None, message_id: Optional[int] = None, inline_message_id: Optional[str] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -1455,7 +1455,7 @@ async def edit_message_text( inline_message_id: Optional[str] = None, parse_mode: ODVInput[str] = DEFAULT_NONE, disable_web_page_preview: ODVInput[bool] = DEFAULT_NONE, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, entities: Optional[Sequence["MessageEntity"]] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, @@ -2401,7 +2401,7 @@ async def send_game( game_short_name: str, disable_notification: DVInput[bool] = DEFAULT_NONE, reply_to_message_id: Optional[int] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, allow_sending_without_reply: ODVInput[bool] = DEFAULT_NONE, protect_content: ODVInput[bool] = DEFAULT_NONE, message_thread_id: Optional[int] = None, @@ -2450,7 +2450,7 @@ async def send_invoice( is_flexible: Optional[bool] = None, disable_notification: DVInput[bool] = DEFAULT_NONE, reply_to_message_id: Optional[int] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, provider_data: Optional[Union[str, object]] = None, send_phone_number_to_provider: Optional[bool] = None, send_email_to_provider: Optional[bool] = None, @@ -3296,7 +3296,7 @@ async def stop_message_live_location( chat_id: Optional[Union[str, int]] = None, message_id: Optional[int] = None, inline_message_id: Optional[str] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, diff --git a/tests/test_official.py b/tests/test_official.py index b39987cea38..3cd826ffe42 100644 --- a/tests/test_official.py +++ b/tests/test_official.py @@ -370,17 +370,13 @@ def check_param_type( ForwardRef(tg_param_type), tg_param_type, # for some reason, some annotations are just a string. ) - print("len mapped 0 ", mapped_type) elif len(mapped) == 1: - print("len mapped 1 ", mapped) mapped_type = mapped.pop() # Resolve nested annotations to get inner types. if (ptb_annotation := list(get_args(ptb_param.annotation))) == []: ptb_annotation = ptb_param.annotation # if it's not nested, just use the annotation - print(f"{tg_param_type=} ||| {mapped_type=} ||| {ptb_annotation=}") - if isinstance(ptb_annotation, list): # Some cleaning: # Remove 'Optional[...]' from the annotation if it's present. We do it this way since: 1) @@ -388,13 +384,11 @@ def check_param_type( # 2) we want to check if our `obj` is same as API's `obj`, and since python evaluates # `Optional[obj] != obj` we have to remove the Optional, so that we can compare the two. if type(None) in ptb_annotation: - print(f"found None in {ptb_param.name}") ptb_annotation.remove(type(None)) # Cleaning done... now let's put it back together. # Join all the annotations back (i.e. Union) ptb_annotation = _unionizer(ptb_annotation, False) - print(ptb_annotation, "after unionizer") # Last step, we need to use get_origin to get the original type, since using get_args # above will strip that out. @@ -404,13 +398,10 @@ def check_param_type( if "collections.abc.Sequence" in str(wrapped): wrapped = Sequence ptb_annotation = wrapped[ptb_annotation] - print(wrapped, "after wrapped", ptb_annotation) # We have put back our annotation together after removing the NoneType! # Now let's do the checking, starting with "Array of ..." types. if "Array of " in tg_param_type: - print("array of ", ptb_param.annotation) - assert mapped_type is Sequence # For exceptions just check if they contain the annotation if ptb_param.name in ARRAY_OF_EXCEPTIONS: @@ -425,22 +416,18 @@ def check_param_type( array_of_mapped: set[type] = _get_params_base(obj_str, TYPE_MAPPING) if len(array_of_mapped) == 0: # no match found, it's from telegram module - print("from tg module") # it could be a list of objects, so let's check that: objs = _extract_words(obj_str) # let's unionize all the objects, with and without ForwardRefs. unionized_objs: list[type] = [_unionizer(objs, True), _unionizer(objs, False)] else: - print("from TYPE_MAPPING") unionized_objs = [array_of_mapped.pop()] # This means it is Array of Array of [obj] if "Array of Array of" in tg_param_type: - # print('isinstance 1', Sequence[Sequence[obj]], ptb_annotation) return any(Sequence[Sequence[o]] == ptb_annotation for o in unionized_objs) # This means it is Array of [obj] - # print('isinstance 2', mapped_type[obj], ptb_annotation) return any(mapped_type[o] == ptb_annotation for o in unionized_objs) # Special case for when the parameter is a default value parameter @@ -451,7 +438,6 @@ def check_param_type( parsed = param_type[mapped_type] if ptb_annotation == parsed: return True - # print(f"cause {mapped_type} != {ptb_annotation} for {ptb_param.name}") return False # Special case for send_* methods where we accept more types than the official API: @@ -460,7 +446,6 @@ def check_param_type( and not isinstance(mapped_type, tuple) and obj.__name__.startswith("send") ): - print("additional_types ") mapped_type = mapped_type | ADDITIONAL_TYPES[ptb_param.name] for (param_name, expected_class), exception_type in EXCEPTIONS.items(): @@ -468,7 +453,17 @@ def check_param_type( ptb_annotation = exception_type # Special case for datetimes - if re.search(r"([_]+|\b)date[^\w]*\b", ptb_param.name) or "Unix time" in tg_parameter[-1]: + if ( + re.search( + r"""([_]+|\b) # check for word boundary or underscore + date # check for "date" + [^\w]*\b # optionally check for a word after 'date' + """, + ptb_param.name, + re.VERBOSE, + ) + or "Unix time" in tg_parameter[-1] + ): # TODO: Remove this in v22 when it becomes a datetime datetime_exceptions = { "file_date", @@ -485,7 +480,6 @@ def check_param_type( if mapped_type == ptb_annotation: return True - print(f"cause {mapped_type=} != {ptb_annotation=} for {ptb_param.name}") return False From e8c71a5fe179fe6081581b84817ed4793036c2af Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Fri, 15 Sep 2023 20:14:16 +0400 Subject: [PATCH 14/15] simplify return statment --- tests/test_official.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/test_official.py b/tests/test_official.py index 3cd826ffe42..5cbf3e98b80 100644 --- a/tests/test_official.py +++ b/tests/test_official.py @@ -477,10 +477,7 @@ def check_param_type( if isinstance(mapped_type, tuple) and any(ptb_annotation == t for t in mapped_type): return True - if mapped_type == ptb_annotation: - return True - - return False + return mapped_type == ptb_annotation def _extract_words(text: str) -> set[str]: From eb9d001ebfd2915e7c253de251fbb8c2c01c69da Mon Sep 17 00:00:00 2001 From: Harshil <37377066+harshil21@users.noreply.github.com> Date: Fri, 15 Sep 2023 20:33:07 +0400 Subject: [PATCH 15/15] fix tests --- telegram/_message.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/telegram/_message.py b/telegram/_message.py index 6cfd21fbcda..74677d7d04b 100644 --- a/telegram/_message.py +++ b/telegram/_message.py @@ -2504,7 +2504,7 @@ async def edit_text( text: str, parse_mode: ODVInput[str] = DEFAULT_NONE, disable_web_page_preview: ODVInput[bool] = DEFAULT_NONE, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, entities: Optional[Sequence["MessageEntity"]] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, @@ -2550,7 +2550,7 @@ async def edit_text( async def edit_caption( self, caption: Optional[str] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, parse_mode: ODVInput[str] = DEFAULT_NONE, caption_entities: Optional[Sequence["MessageEntity"]] = None, *, @@ -2597,7 +2597,7 @@ async def edit_caption( async def edit_media( self, media: "InputMedia", - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -2681,7 +2681,7 @@ async def edit_live_location( self, latitude: Optional[float] = None, longitude: Optional[float] = None, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, horizontal_accuracy: Optional[float] = None, heading: Optional[int] = None, proximity_alert_radius: Optional[int] = None, @@ -2731,7 +2731,7 @@ async def edit_live_location( async def stop_live_location( self, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE, @@ -2886,7 +2886,7 @@ async def delete( async def stop_poll( self, - reply_markup: Optional[InlineKeyboardMarkup] = None, + reply_markup: Optional["InlineKeyboardMarkup"] = None, *, read_timeout: ODVInput[float] = DEFAULT_NONE, write_timeout: ODVInput[float] = DEFAULT_NONE,