Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API 4.5 #1508

Merged
merged 38 commits into from
Mar 28, 2020
Merged

API 4.5 #1508

merged 38 commits into from
Mar 28, 2020

Conversation

Bibo-Joshi
Copy link
Member

@Bibo-Joshi Bibo-Joshi commented Sep 6, 2019

PR for the API 4.5 changes. Closes #1689

  • Two new messageEntities, strikethrough and underline
  • nested messageEntities
  • New parsemode, MardownV2
  • New file_unique_id for Animation, Audio, Document, PassportFile, PhotoSize, Sticker, Video, VideoNote, Voice, File
  • small_file_unique_id and big_file_unique_id to the object ChatPhoto.
  • custom_title for ChatMember
  • new method setChatAdministratorCustomTitle
  • new slow_mode_delay for Chat object
  • Before merge, merge master and revert temporarily skip tests failing b/c missing api 4.5 #1738
  • When merged, update this wiki page.

Original description:

Cherry-picked from #1464 since we're not sure yet how Telegram will actually handle nested entities in API 4.5.

This only allows something like _bold *within* italic_ and not *bold _overlapping* italic_. The announcement is not every clear, if that is also allowed …

Leaving this here as a reminder, that we already have some code for that case.

dmytrohoi and others added 2 commits January 3, 2020 01:45
Changes:

 - custom_title for ChatMember
 - new method setChatAdministratorCustomTitle for Bot
 - new slow_mode_delay for Chat

Update due to new API future `custom_title` from API 4.5 (https://core.telegram.org/bots/api#december-31-2019)
@Bibo-Joshi Bibo-Joshi changed the title API 4.5: Nested entities API 4.5 Jan 3, 2020
@Bibo-Joshi
Copy link
Member Author

The PR is ready for review now. The test failures seem unrelated (TestBot.test_pin_and_unpin_message and TestConversationHandler.test_per_message_false_warning_is_only_shown_once are nothing we fiddled with)

@Bibo-Joshi Bibo-Joshi requested review from Eldinnie and tsnoam and removed request for Eldinnie January 8, 2020 19:29
Copy link
Contributor

@dmytrohoi dmytrohoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewers, what do you think:

  1. Should we add a shortcut for bot.set_chat_administrator_custom_title() to ChatMember class from chatmember.py?
  2. I think because of this description, This is an obsolete mode saved for backward compatibility. To use this mode, pass Markdown to the parse_mode field from chapter "Markdown" in Telegram Bot API documentation, we need to use MarkdownV2 as * default * option in all related to markdown functions.
  3. Instead of the markdown_smt_v2 statements, perhaps we need to use the markdown_v2_smt? (one of them is commented in this review)

telegram/message.py Outdated Show resolved Hide resolved
@Bibo-Joshi
Copy link
Member Author

  1. Should we add a shortcut for bot.set_chat_administrator_custom_title() to ChatMember class from chatmember.py?
  2. I think because of this description, This is an obsolete mode saved for backward compatibility. To use this mode, pass Markdown to the parse_mode field from chapter "Markdown" in Telegram Bot API documentation, we need to use MarkdownV2 as * default * option in all related to markdown functions.
  3. Instead of the markdown_smt_v2 statements, perhaps we need to use the markdown_v2_smt? (one of them is commented in this review)
  1. and 3. sound good to me. Could do it in the next days, but if you want to, please go ahead.

  2. There is no default value for parse_mode anywhere, there is only Message.reply_markdown (correct me, if I'm missing something). For the latter, there is Message.reply_markdown_v2 now. For the former, Add default values #1490 is the PR, but it's left to the user to choose a default value, which makes sense because just putting MarkdownV2 as default would break backwards compitibility since it requires more characters to be escaped than v1

@Eldinnie
Copy link
Member

Overall looking good. Just some questions and remarks added

@Eldinnie
Copy link
Member

Codecov, mainly is unhappy because of the added checks for py2 (in parse_entities) which are never hit, because we don't test 2.7 anymore. You can ignore it

Copy link
Member Author

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review @Eldinnie! Will try to incorporate the changes on the WE at the latest

tests/test_animation.py Show resolved Hide resolved
tests/test_animation.py Show resolved Hide resolved
tests/test_message.py Outdated Show resolved Hide resolved
Copy link
Member

@Bibo-Joshi Since we want to get this done asap, I guess I can chime in this afternoon for me, if thats alright with you

@Bibo-Joshi
Copy link
Member Author

Latest commit closes #1654 . Put this here since we're fiddling with _parse_markdown anyway and we would have merge conflicts else …

@Poolitzer Poolitzer mentioned this pull request Feb 17, 2020
2 tasks
@gpchelkin
Copy link

Just a friendly reminder, I think it better belongs here. Please bump supported Bot API version number in https://github.com/python-telegram-bot/python-telegram-bot/blob/master/README.rst

It says 4.1 now, but one could tell from the changelogs, that actually it is 4.4 now. I think it might be repelling or misleading for new users of this great wrapper :-)

@Bibo-Joshi
Copy link
Member Author

Bibo-Joshi commented Mar 6, 2020

pytest failures seem unrelated (test_conversation_timeout_cancel_conflict)
codecov complains that in Message._parse_*, the parts for if sys.maxunicode == 0xffff are not hit, but I woudn't know how to test that …

@Bibo-Joshi Bibo-Joshi mentioned this pull request Mar 28, 2020
@Bibo-Joshi Bibo-Joshi merged commit 8d2c7af into master Mar 28, 2020
@Bibo-Joshi Bibo-Joshi deleted the nested-entities branch March 28, 2020 17:13
blueset added a commit to ehForwarderBot/efb-telegram-master that referenced this pull request Apr 3, 2020
…ique_id`` attribute.

NOTE: this will only work when `PTB supports Bot API 4.5`__.

__ python-telegram-bot/python-telegram-bot#1508
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4.5 bot api update arrived
5 participants