-
Notifications
You must be signed in to change notification settings - Fork 5.9k
Input media subclasses init handling refactor #2573 #2717
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
Input media subclasses init handling refactor #2573 #2717
Conversation
…ryptionError (python-telegram-bot#2621) * move telegramdecryptionerror to error.py * Change error class name
…t#2612) * feat: add docs about docs * fix: improve looks * fix: make link work * fix: this looks better * Improved markdown, updated link * Less justifying Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
* Fix incomplete type annotations for CallbackContext Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
* Feat: Custom pytest marker Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
* Make basepersistence methods abstractmethod Signed-off-by: starry69 <starry369126@outlook.com> Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
…m-bot#2634) Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com> Co-authored-by: poolitzer <25934244+Poolitzer@users.noreply.github.com>
…ring InputMedia and subclasses
# Conflicts: # telegram/bot.py # telegram/ext/callbackcontext.py # telegram/ext/dispatcher.py # telegram/ext/extbot.py # telegram/ext/jobqueue.py # tests/conftest.py # tests/test_bot.py
|
the tests here are failing in pre-commit due to the we discussed. any solution for this? |
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.
Hi. Thanks for the PR! I left a number of comments below. In addition to that:
- IMO
_Base(Thumb)Mediumcan both go into the same filebasemedium.py - about mypy fails: in the reported lines we access something like
input_media_object.parse_mode, but because we only know thatisinstance(i_m_object, InputMedia)and up to nowInputMediahas no attributeparse_mode, mypy threw an error. this is resolved by your PR, so thetype: ignorecomments are no longer needed - you can remove them - Please double check that the docs of
Audio/Documentetc show display the methodsget_fileandto/de_json(everything that's overridden in_Base(Thumb)Medium. The contribution guide has details on how to build the docs locally. You may need to add:inherited-members:to the filesdocs/source/telegram.audio/document/….rst, see here - 5th bullet point in "options and advanced usage" - I'm not entirely sure what's going on with the signature checks. Will have to look at that with more detail, which might take a few days
- test_official is failing because
InputMediahas no documented attributes, but we added some. Please add another exception to this logic
🤦♂️ |
in the contribution guideline it says to run this: |
|
Except for |
test_official is fixed. |
@Bibo-Joshi can you maybe show me how the entire docstring should be written? |
|
@eldbud sorry, should have been more specific. The docs for the class |
@Bibo-Joshi i believe i am done. if not more comments, can this pr be approved? |
harshil21
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.
looks good!
|
Thanks for this contribution! :) |
InputMedia.__init__so that it initializes the attributes common to all deriving classes.InputMedia._get_thumb_objectstatic method_BaseMediumclass, base class for media objects_BaseThumbedMediumclass, base class for media objects that may have a thumbnail._BaseMedium/_BaseThumbedMediumcloses #2573