-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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? |
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)Medium
can 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 nowInputMedia
has no attributeparse_mode
, mypy threw an error. this is resolved by your PR, so thetype: ignore
comments are no longer needed - you can remove them - Please double check that the docs of
Audio/Document
etc show display the methodsget_file
andto/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
InputMedia
has 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? |
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_object
static method_BaseMedium
class, base class for media objects_BaseThumbedMedium
class, base class for media objects that may have a thumbnail._BaseMedium
/_BaseThumbedMedium
closes #2573