Thanks for the nice PR! I left a new nitpicks below.
- imo, the
quoteargument of the shortcut methods inMessage,Chat,Usershould also be keyword only since its added by PTB. But lets wait and see what the other maintainers think.
I guess the need to separate that from the other arguments is not as big, but I'm okay with the idea. We should then however make all PTB-added arguments keyword-only. We have have few more of those, e.g. InlineQurey.answer(…, auto_pagination=True) and Bot.send_location(…, location=…). For the methods of Bot, those can be seen here. For the shortcut methods, that would be all the arguments that are passed check_shortcut_signature(…, addition_kwargs=[…]) in the tests, e.g. here.
Inserting a print(shortcut, additional_kwargs) here & then running the test suite once should reveal all of them.
Corresponding .. versionchanged:: 20.0 directives should be added.
About tests:
- In
test_bot.py, one could add a test to check all bot methods and see if there are exactly 5 keyword arguments by checking theirkindandname({read, write, connect, pool}_timeoutandapi_kwargs). One could retrieve all public bot methods by usingiscoroutinefunctionand checking if the method doesn't start with an underscore.- The same would be need to done for the shortcuts, i.e in
test_{chat, user, message}
I think we can get away more easily if we make all ptb-added parameters kwargs-only:
- For checking the signature of bot methods, we can just check in
test_officialthat all parameters not documented by TG are are kwargs-only. - For checking shortcut signatures, it would suffice to ensure that the
additional_kwargsare kwargs-only. all other parameters are matched against the Bot signature anyway.
@harshil21 what do you think?
@tal66 I'm aware that unit tests can be tedious to wrap your head around if you're not entirely familiar with the code base. If you have troubles with updating tests, please don't hesitate to reach out either here or in the dev-group on Telgeram!