Input media subclasses init handling refactor #2573 by eldbud · Pull Request #2717 · python-telegram-bot/python-telegram-bot

eldbud · 2021-10-08T13:22:33Z

Refactored InputMedia.__init__ so that it initializes the attributes common to all deriving classes.
Add InputMedia._get_thumb_object static method
Created _BaseMedium class, base class for media objects
Created _BaseThumbedMedium class, base class for media objects that may have a thumbnail.
Refactored Media classes so that they inherit from _BaseMedium/_BaseThumbedMedium

…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>

)

…am-bot#2619, python-telegram-bot#2636)

…m-bot#2634) Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>

python-telegram-bot#2657)

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

…tor-python-telegram-bot#2573

eldbud · 2021-10-09T12:27:56Z

the tests here are failing in pre-commit due to the

telegram/bot.py:232: error: unused 'type: ignore' comment
telegram/bot.py:233: error: unused 'type: ignore' comment
telegram/ext/extbot.py:162: error: unused 'type: ignore' comment
telegram/ext/extbot.py:163: error: unused 'type: ignore' comment
Found 4 errors in 2 files (checked 149 source files)

we discussed. any solution for this?

Bibo-Joshi

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 file basemedium.py
about mypy fails: in the reported lines we access something like input_media_object.parse_mode, but because we only know that isinstance(i_m_object, InputMedia) and up to now InputMedia has no attribute parse_mode, mypy threw an error. this is resolved by your PR, so the type: ignore comments are no longer needed - you can remove them
Please double check that the docs of Audio/Document etc show display the methods get_file and to/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 files docs/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

telegram/files/animation.py

telegram/files/basemedium.py

telegram/files/inputmedia.py

telegram/telegramobject.py

eldbud · 2021-10-11T18:28:19Z

You probably need to convert the type hint into a string: def foo(… bot: 'Bot').

🤦‍♂️
of course, they're under if TYPE_CHECKING, foolishly forgot that. Thank you!

eldbud · 2021-10-11T18:40:19Z

Ah, yes. test official only runs if you have an environment variable set. the contribution guide has some details about that :)

in the contribution guideline it says to run this: export TEST_OFFICIAL=true prior to running tests.
Because i am on windows computer, i replace "export" (which does not work in windows) with "set" (set TEST_OFFICIAL=true),
but it didn't help, i am still getting "test_official is not enabled".
What should i do?

Bibo-Joshi · 2021-10-12T06:34:58Z

Except for test_official, this LGTM now :) If you can't get the environment variable to work, you can also just temporarily delete the @pytest.mark.skipif decorator on test_official/test_official (at the very bottom). Note that pycharm also allows you to set environment configs via the run configurations.

eldbud · 2021-10-12T17:14:38Z

Except for test_official, this LGTM now :)

test_official is fixed.
last item left - documentation of attributes in subclasses

eldbud · 2021-10-12T17:22:44Z

You can do something like

telegram.Audio
==============

.. Also lists methods of _Base(Thumb)Medium, but not the ones of TelegramObject

.. autoclass:: telegram.Audio
    :members:
    :show-inheritance:
    :inherited-members: TelegramObject

where the line .. Also lists … is a comment

the ..autoclass::telegram.Audio line raises an error: C:\Users\eldad\PycharmProjects\python-telegram-bot\telegram\_files\audio.py:docstring of telegram._files.audio.Audio:1: WARNING: duplicate object description of telegram._files.audio.Audio, other instance in telegram.audio, use :noindex: for one of them
without it, methods still don't appear.

@Bibo-Joshi can you maybe show me how the entire docstring should be written?

Bibo-Joshi · 2021-10-13T06:18:11Z

@eldbud sorry, should have been more specific. The docs for the class telegram.Audio are generated by the file docs/source/telegram.audio.rst. You can adapat that file by adding the directive option :inherited-members: TelegramObject to the .. autoclass directive and by adding the comment. Similar for all other classes that inherit from _Base(Thumb)Medium

eldbud · 2021-10-14T21:08:46Z

@eldbud sorry, should have been more specific. The docs for the class telegram.Audio are generated by the file docs/source/telegram.audio.rst. You can adapat that file by adding the directive option :inherited-members: TelegramObject to the .. autoclass directive and by adding the comment. Similar for all other classes that inherit from _Base(Thumb)Medium

@Bibo-Joshi i believe i am done. if not more comments, can this pr be approved?

harshil21

looks good!

Bibo-Joshi · 2021-10-15T16:04:22Z

Thanks for this contribution! :)

Bibo-Joshi and others added 26 commits September 15, 2021 17:09

Temporarily enable tests for the v14 branch

331c4f5

Move and Rename TelegramDecryptionError to telegram.error.PassportDec…

9dc30cf

…ryptionError (python-telegram-bot#2621) * move telegramdecryptionerror to error.py * Change error class name

Improve Type Hinting for CallbackContext (python-telegram-bot#2587)

3a19383

* Fix incomplete type annotations for CallbackContext Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>

Add Custom pytest Marker to Ease Development (python-telegram-bot#2628)

414a18c

* Feat: Custom pytest marker Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>

Make BasePersistence Methods Abstract (python-telegram-bot#2624)

8edb49d

* Make basepersistence methods abstractmethod Signed-off-by: starry69 <starry369126@outlook.com> Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>

Refactor Initialization of Persistence Classes (python-telegram-bot#2604

cce9c18

)

Remove __dict__ from __slots__ and drop Python 3.6 (python-telegr…

dc0207a

…am-bot#2619, python-telegram-bot#2636)

Remove day_is_strict argument of JobQueue.run_monthly (python-telegra…

4af252f

…m-bot#2634) Co-authored-by: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>

Drop Non-CallbackContext API (python-telegram-bot#2617)

b6e4b54

Fix Signatures and Improve test_official (python-telegram-bot#2643)

4359812

Remove Deprecated Functionality (python-telegram-bot#2644)

dbdd35e

Add User Friendly Type Check For Init Of {Inline, Reply}KeyboardMarkup (

a7c7c82

python-telegram-bot#2657)

Refine Dispatcher.dispatch_error (python-telegram-bot#2660)

8ac65fc

Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>

Warnings Overhaul (python-telegram-bot#2662)

66f0604

Clear Up Import Policy (python-telegram-bot#2671)

4f21c06

Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>

Make InlineQuery.answer Raise ValueError (python-telegram-bot#2675)

5542a46

Doc Fixes (python-telegram-bot#2597)

2cf9311

Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com> Co-authored-by: poolitzer <25934244+Poolitzer@users.noreply.github.com>

created _MediumBase class and media attribute mixins classes, refacto…

c966041

…ring InputMedia and subclasses

Merge remote-tracking branch 'upstream/v14' into v14

bcc62bb

# 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

Merge branch 'v14' into InputMedia-subclasses-__init__-handling-refac…

7dc3e56

…tor-python-telegram-bot#2573

committing hopefully

d0eb6d4

Merge remote-tracking branch 'upstream/v14' into v14

9ed515a

Merge branch 'v14' into InputMedia-subclasses-__init__-handling-refac…

800603a

…tor-python-telegram-bot#2573

added ver doc

898fe00

added ver doc

e2b7a4a

Bibo-Joshi self-requested a review October 8, 2021 16:49

Bibo-Joshi mentioned this pull request Oct 9, 2021

[REFACTOR]: Enum constants #2708

Merged

5 tasks

Bibo-Joshi requested changes Oct 9, 2021

View reviewed changes

attending to review notes

6d04075

eldbud added 6 commits October 11, 2021 21:41

attending to review notes

20765d7

Merge remote-tracking branch 'upstream/v14' into v14

d82473a

merged from v14

b6ea24d

made base media modules protected

d83b4f2

closes python-telegram-bot#2573

bab0f54

closes python-telegram-bot#2573

c1cb104

Bibo-Joshi linked an issue Oct 12, 2021 that may be closed by this pull request

Refactor how InputMedia* subclasses handle inits #2573

Closed

harshil21 added enhancement 🛠 refactor change type: refactor labels Oct 12, 2021

harshil21 added this to the v14 milestone Oct 12, 2021

fixed test_official[InputFile]

f688d91

fixed docs generation to include methods

13e7dfb

eldbud requested review from Bibo-Joshi and harshil21 October 14, 2021 21:54

harshil21 approved these changes Oct 15, 2021

View reviewed changes

Bibo-Joshi merged commit 38a6a6d into python-telegram-bot:v14 Oct 15, 2021

eldbud deleted the InputMedia-subclasses-__init__-handling-refactor-#2573 branch October 15, 2021 22:04

github-actions bot locked and limited conversation to collaborators Oct 17, 2021

harshil21 added the hacktoberfest-accepted other: hacktoberfest-accepted label Oct 30, 2021

Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024

Conversation

eldbud commented Oct 8, 2021 • edited by Bibo-Joshi Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eldbud commented Oct 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bibo-Joshi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eldbud commented Oct 11, 2021

Uh oh!

eldbud commented Oct 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bibo-Joshi commented Oct 12, 2021

Uh oh!

eldbud commented Oct 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eldbud commented Oct 12, 2021

Uh oh!

Bibo-Joshi commented Oct 13, 2021

Uh oh!

eldbud commented Oct 14, 2021

Uh oh!

harshil21 left a comment

Choose a reason for hiding this comment

Uh oh!

Bibo-Joshi commented Oct 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

eldbud commented Oct 8, 2021 •

edited by Bibo-Joshi

Loading

eldbud commented Oct 9, 2021 •

edited

Loading

eldbud commented Oct 11, 2021 •

edited

Loading

eldbud commented Oct 12, 2021 •

edited

Loading