FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Api 4.8 by Bibo-Joshi · Pull Request #1917 · python-telegram-bot/python-telegram-bot · GitHub

Api 4.8 - #1917

Merged
Bibo-Joshi merged 7 commits into
masterfrom
api-4.8
May 2, 2020
Merged

Api 4.8#1917
Bibo-Joshi merged 7 commits into
masterfrom
api-4.8

Conversation

Bibo-Joshi commented Apr 24, 2020
edited
Loading

Copy link
Copy Markdown
Member

Pure API changes

  • Bot.send_poll: explanation, explanation_parse_mode,
  • Poll.{explanation, explanation_entities, open_period, close_date}
  • Bot.send_dice: emoji (for whatever reason, sending a 'darts' is considered a die now …
  • Dice.emoji

Convenience stuff

  • Poll.parse_explanation_entity/ies
  • Filters.dice.dice and Filters.dice.darts for filtering only the corresponding emojis. Allow the Filters.dice.*([1, 2]) syntax for both

Todo

  • Release notification should maybe include a statemant, that Dice a second positional argument.
  • Telegram seems to have changed something with the photos. Getting telegram.error.BadRequest: Type of file mismatch. Need to investigate

Bibo-Joshi added the ⚙️ bot-api affected functionality: bot-api label Apr 24, 2020

Copy link
Copy Markdown
Member

@Bibo-Joshi Last time I faced this error in the support groups I suggested checking the dimensions of it. Maybe try uploading said photo as document, if that works, that might be it?

Copy link
Copy Markdown
Member Author

@Poolitzer Problem seems to be sending a photo via

with open('tests/data/thumb.jpg', 'rb') as f:
    bot.send_photo(chat_id, f)

Problem persists, if I swat tests/data/telegram.jpg with telegram.png or thumb.jpg. However, I have no problems sending the to myself with a personal bot(token). Could that be a type of flood control? We have been getting some flooding errors in the tests recently … @tsnoam maybe we can add new/replace the existing tests bots?

Poolitzer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

great PR, slight improvement

Comment thread telegram/dice.py Outdated

Copy link
Copy Markdown
Member

One thing: What about adding the emojis from Dice as constants? We normally do that. Right now we only have a list.

Copy link
Copy Markdown
Member Author

One thing: What about adding the emojis from Dice as constants? We normally do that. Right now we only have a list.

Added that. Still kept the ALL_EMOJI list, though

Copy link
Copy Markdown
Member

@Bibo-Joshi Sure, didnt complain about that ;P

Copy link
Copy Markdown
Member Author

CI Fail: test_max_caption_length gives an Internal server error for some reason. seems unrelated.
Coverage: can't test utf-16 encoding, that's basically it. Still, codecov is satisfied.

tsnoam left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

In general looks good. A small comment on docstring.

However, I did not understand the following action item in the PR description:

Release notification should maybe include a statemant, that Dice a second positional argument.

Comment thread telegram/bot.py Outdated
Co-authored-by: Noam Meltzer <tsnoam@gmail.com>
Bibo-Joshi merged commit c7c56ad into master May 2, 2020
Bibo-Joshi deleted the api-4.8 branch May 2, 2020 09:56
Bibo-Joshi mentioned this pull request May 12, 2020
6 tasks
github-actions Bot locked as resolved and limited conversation to collaborators Aug 17, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

⚙️ bot-api affected functionality: bot-api

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL