Skip to content

API 4.5#1508

Merged
Bibo-Joshi merged 38 commits intomasterfrom
nested-entities
Mar 28, 2020
Merged

API 4.5#1508
Bibo-Joshi merged 38 commits intomasterfrom
nested-entities

Conversation

@Bibo-Joshi
Copy link
Member

@Bibo-Joshi Bibo-Joshi commented Sep 6, 2019 •
edited
Loading

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.

@Bibo-Joshi Bibo-Joshi mentioned this pull request Sep 6, 2019
Merged
@Bibo-Joshi Bibo-Joshi added the ⚙️ bot-api affected functionality: bot-api label Sep 6, 2019
@Eldinnie Eldinnie added the 📋 do-not-merge-yet work status: do-not-merge-yet label Sep 9, 2019
@Bibo-Joshi Bibo-Joshi mentioned this pull request Jan 2, 2020
Closed
8 tasks
dmytrohoi and others added 2 commits January 3, 2020 01:45
@dmytrohoi
Update to custom_title feature and slow_mode_delay option
8816d5a 
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
Merge remote-tracking branch 'origin' into nested-entities
a58188d
@Bibo-Joshi Bibo-Joshi changed the title API 4.5: Nested entities API 4.5 Jan 3, 2020
@Bibo-Joshi Bibo-Joshi removed the 📋 do-not-merge-yet work status: do-not-merge-yet label Jan 3, 2020
@Bibo-Joshi
Copy link
Member Author

Bibo-Joshi commented Jan 8, 2020

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
dmytrohoi
dmytrohoi reviewed Jan 10, 2020
View reviewed changes
Copy link
Contributor

@dmytrohoi dmytrohoi left a comment •
edited
Loading

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)

@Bibo-Joshi
Copy link
Member Author

Bibo-Joshi commented Jan 10, 2020

  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

Eldinnie commented Feb 10, 2020

Overall looking good. Just some questions and remarks added

@Eldinnie
Copy link
Member

Eldinnie commented Feb 10, 2020

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

Bibo-Joshi
Bibo-Joshi commented Feb 10, 2020
View reviewed changes
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

@Poolitzer Telegram GithubBot Revised
Copy link
Member

Poolitzer commented Feb 10, 2020

@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 Bibo-Joshi mentioned this pull request Feb 17, 2020
Closed
@Bibo-Joshi
Copy link
Member Author

Bibo-Joshi commented Feb 17, 2020

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
Merged
2 tasks
@gpchelkin
Copy link

gpchelkin commented Mar 1, 2020

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 •
edited
Loading

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
Closed
@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
@blueset
fix: detection of updated file is fixed by the inclusion of ``file_un…
0ff72cc 
…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.

Reviewers

@Poolitzer Poolitzer Poolitzer left review comments

@Eldinnie Eldinnie Eldinnie requested changes

@tsnoam tsnoam Awaiting requested review from tsnoam

+1 more reviewer

@dmytrohoi dmytrohoi dmytrohoi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

⚙️ bot-api affected functionality: bot-api

Projects

None yet

Milestone

12.4.3

Development

Successfully merging this pull request may close these issues.

4.5 bot api update arrived

5 participants

@Bibo-Joshi @Eldinnie @Poolitzer @gpchelkin @dmytrohoi