TelegramObject.api_kwargs
#3233
Conversation
Bibo-Joshi
added
🛠 refactor
There was a problem hiding this comment.
Nice job, must have been tedious to change for all classes manually. Some observations:
- Should we make the
api_kwargsargument private in all classes since it's not documented? Review related: MissingSee review comment no. 21 first.versionchangeddoc for many inlinequery classes, I marked 2 of them.- test_official should be updated too.
Btw do you want to also do #3146 in this or another PR?
# Conflicts: # docs/substitutions/global.rst
|
Sorry, forgot to reply on this 🙈
IISC We usually don't re-document parameters of the parent class 🤔 I wouldn't consider it private.
What needs updating here?
That's rather independent IMO. I'm not planning on doing that in this PR |
Oops, forgot that this is documented in TGObject
only the
alright |
harshil21
left a comment
harshil21
left a comment
There was a problem hiding this comment.
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?
…nd remove some redundant inits
Poolitzer
left a comment
Poolitzer
left a comment
There was a problem hiding this comment.
I went through them, great change, looks good to me!
# Conflicts: # telegram/ext/_extbot.py
First part of #2698.
In this context also
TelegramObjecta proper__init__that's called in the subclassesbotand**(_)kwargsarguments from the subclassesset_botfrom the subclasses__init__tode_jsonDocumentation updates coming up soonish
Checklist for PRs
.. versionadded:: version,.. versionchanged:: versionor.. deprecated:: versionto the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)Added myself alphabetically toAUTHORS.rst(optional)Added new classes & modules to the docs and all suitable__all__s