Member

First part of #2698.

In this context also

• gives TelegramObject a proper __init__ that's called in the subclasses
• Removes all bot and **(_)kwargs arguments from the subclasses
• moves calling set_bot from the subclasses __init__ to de_json

Documentation updates coming up soonish

Checklist for PRs

• Added .. versionadded:: version, .. versionchanged:: version or .. deprecated:: version to the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)
• Created new or adapted existing unit tests
• Documented code changes according to the CSI standard
• Added myself alphabetically to AUTHORS.rst (optional)
• Added new classes & modules to the docs and all suitable __all__ s

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job, must have been tedious to change for all classes manually. Some observations:

1. Should we make the api_kwargs argument private in all classes since it's not documented?
2. Review related: Missing versionchanged doc for many inlinequery classes, I marked 2 of them. See review comment no. 21 first.
3. test_official should be updated too.

Btw do you want to also do #3146 in this or another PR?

Member Author

Sorry, forgot to reply on this 🙈

1. Should we make the api_kwargs argument private in all classes since it's not documented?

IISC We usually don't re-document parameters of the parent class 🤔 I wouldn't consider it private.

3. test_official should be updated too.

What needs updating here?

Btw do you want to also do #3146 in this or another PR?

That's rather independent IMO. I'm not planning on doing that in this PR

Member

IISC We usually don't re-document parameters of the parent class 🤔 I wouldn't consider it private.

Oops, forgot that this is documented in TGObject

What needs updating here?

only the _kwargs and kwargs in this variable

That's rather independent IMO. I'm not planning on doing that in this PR

alright

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, the new tests give more confidence that it works.

Also just realized (sorry should've noticed sooner), but shouldn't we make api_kwargs keyword only to better match the behaviour we have for bot methods api_kwargs?

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.