-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Description
Currently we have a few places, where the default value in the signature specifies the default value specified by Telegram. This is both the case for bot methods and Telegram classes. E.g. in send_poll
:
python-telegram-bot/telegram/bot.py
Lines 5141 to 5147 in 92cb6f3
def send_poll( | |
self, | |
chat_id: Union[int, str], | |
question: str, | |
options: List[str], | |
is_anonymous: bool = True, | |
type: str = Poll.REGULAR, # pylint: disable=W0622 |
and in ReplyKeyboardMarkup
:
python-telegram-bot/telegram/replykeyboardmarkup.py
Lines 87 to 92 in 92cb6f3
def __init__( | |
self, | |
keyboard: Sequence[Sequence[Union[str, KeyboardButton]]], | |
resize_keyboard: bool = False, | |
one_time_keyboard: bool = False, | |
selective: bool = False, |
More importantly, we pass those values to TG. This isn't really necessary, since we'd get the same behavior when not passing those parameters. This has two effects:
- More params are getting passed -> more traffic than strictly necessary. admittedly, probably barely noticable, but still.
- In case TG changes some defaults, the change will not be reflected in PTB until we change the signature.
While the first one is clearly a downside, one might argue that the second one does have some advantages in terms of stability. Personally, I'd rather comply with the current state of the bot API, but I'm open to discussing this.
In any case, I think we should do this uniformly, i.e. either reflect the default value in the signature everywhere or nowhere & try to unit test this in test_official
. E.g. Currently e.g. the emoji
parameter of send_dice
has None
as default value instead of '🎲'
:
python-telegram-bot/telegram/bot.py
Lines 5315 to 5326 in 92cb6f3
def send_dice( | |
self, | |
chat_id: Union[int, str], | |
disable_notification: ODVInput[bool] = DEFAULT_NONE, | |
reply_to_message_id: int = None, | |
reply_markup: ReplyMarkup = None, | |
timeout: ODVInput[float] = DEFAULT_NONE, | |
emoji: str = None, | |
api_kwargs: JSONDict = None, | |
allow_sending_without_reply: ODVInput[bool] = DEFAULT_NONE, | |
protect_content: bool = None, | |
) -> Message: |
In case we go for removing the default values, some bot methods internals may be simplified.
Either change would be non-breaking since the current defaults in the signatures are up to date with the defaults documented in the API.