feat(Bot): add shortcut to pass caption to media group#3295
Merged
Bibo-Joshi merged 25 commits intopython-telegram-bot:masterfrom Nov 2, 2022
lemontree210:caption-for-media-group-3185
Merged
feat(Bot): add shortcut to pass caption to media group#3295Bibo-Joshi merged 25 commits intopython-telegram-bot:masterfrom lemontree210:caption-for-media-group-3185
Bot): add shortcut to pass caption to media group#3295Bibo-Joshi merged 25 commits intopython-telegram-bot:masterfrom
lemontree210:caption-for-media-group-3185
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
Member
Author
Bibo-Joshi
requested changes
Oct 17, 2022
Member
Bibo-Joshi
left a comment
There was a problem hiding this comment.
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
requested changes
Oct 19, 2022
Member
Bibo-Joshi
left a comment
There was a problem hiding this comment.
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
Bibo-Joshi
reviewed
Oct 20, 2022
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
Bibo-Joshi
reviewed
Oct 23, 2022
Bibo-Joshi
reviewed
Oct 23, 2022
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
Member
|
Thanks again for your work & patience on this feature :) |
Member
Author
|
Thank you! I hope we didn't forget to add |
Member
|
Ah, damn. I'll add that in the doc-fixes branch :) |
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.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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)