-
Notifications
You must be signed in to change notification settings - Fork 5.9k
feat(Bot): add shortcut to pass caption to media group
#3295
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
feat(Bot): add shortcut to pass caption to media group
#3295
Conversation
closes #3185 add 3 keyword-only arguments to pass caption to entire media group before that a caption for the group had to be manually added to the 1st media item in the group
"group_caption", "group_caption_parse_mode", and "group_caption_entities" are not part of API, so exclude them from check of arguments matching API
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for your nice PR! I left a number of comments - but most are actually just nit-picking on the documentation :D
- replace "images" with "media" - check not only .caption but also .caption_entities and .parse_mode of individual media items
we should try to copy as little as possible: if none of the `group_*` parameters was passed, we don't need copying. Otherwise, we only need to copy (the list and) the first media item rather than all of them
rename to match arguments of functions that handle individual media: - group_caption -> caption - group_caption_parse_mode -> parse_mode - group_caption_entities -> caption_entities
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the updates! I left a few more comments below. Take your time with the defaults stuff (nice to see that you're eager to figure it out yourself!), but feel free to reach out for help if you want it :)
make sure the original media group is intact
check that the error is raised if user passes media that only have `parse_mode` or `caption_entities`
Check if the user explicitly set parse_mode for the InputMedia* object. Even if it is set to None, ValueError must be raised.
add fixture for a media group without individual captions rather than taking `media_group` fixtures and setting attributes manually this fixture can be used later when checking defaults
* make `parse_mode` default to DEFAULT_NONE in `Bot` and shortcut methods in classes * exclude `parse_mode` from check in conftest.py * add tests for handling of default values
to allow multiple calls with same media group, I need to copy not only the first item (that gets the caption) but all items because otherwise bot with Defaults will assign its default parse mode to every item, which will cause ValueError upon subsequent calls unless there are other things to be fixed, this should be the commit that closes #3185
also remove `timeout` as a remnant from v13
`_bot.py` had two imports (import copy and from copy import copy) leave first import and replace `copy()` with `copy.copy()` in code
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome 🥳
|
Thanks again for your work & patience on this feature :) |
|
Thank you! I hope we didn't forget to add |
|
Ah, damn. I'll add that in the doc-fixes branch :) |
closes #3185
Adds 3 keyword-only arguments to pass caption to entire media group.
All tests pass (locally) including
test_official.py.All pre-commit hooks passed except for
pylintthat found errors in code that I hadn't changed (cyclic import intelegram/ext/_chatjoinrequesthandler.py).I'm not sure if I have to:
.. versionadded:: version,.. versionchanged:: versionor.. deprecated:: versionto the docstrings for user facing changes (for methods/class descriptions, arguments and attributes) (I changed doctrings themselves, just not sure about theseversion...notes)Chat,User,Message)