-
Notifications
You must be signed in to change notification settings - Fork 6k
Updating the examples #2937
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
Updating the examples #2937
Conversation
Also put one logging line where it belongs
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.
Hey there. Relax, I am just a little warning for the maintainers to release directly after merging your PR, otherwise we have broken examples and people might get confused :)
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.
Thanks for the PR! changes themself look good except for the pre-commit errors. would you mind adding this section back to the pre-commit config?
python-telegram-bot/.pre-commit-config.yaml
Lines 48 to 59 in d117d21
| - id: mypy | |
| name: mypy-examples | |
| files: ^examples/.*\.py$ | |
| args: | |
| - --no-strict-optional | |
| - --follow-imports=silent | |
| additional_dependencies: | |
| - certifi | |
| - tornado>=6.1 | |
| - APScheduler==3.6.3 | |
| - cachetools==4.2.2 | |
| - . # this basically does `pip install -e .` |
I had removed it temporarily while working on the asyncio changes …
PS: Don't know why codecov complains - no tracked files changed
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.
Hey! Looks like you edited the (dev) requirements or the pre-commit hooks. I'm just a friendly reminder to keep the pre-commit hook versions in sync with the dev requirements and the additional dependencies for the hooks in sync with the requirements :)
|
I added the config and fixed the black thing. Black itself has currently an import issue in the pre-commit and pylint complains locally, so I forced git to commit anyway and am interested in how it runs here. |
|
I found the issue with black psf/black#2964, we either need to pin the click dependency to a lower version or upgrade the black version, what would you prefer and should I squeeze it in here or in a second PR? |
|
@Poolitzer We're updating dependencies in #2925 so imo no need to do it here |
|
@harshil21 I'm okay with doing it here/in asyncio as the CI currently fails otherwise. #2925 is due only after asyncio |
|
@Bibo-Joshi So to be less disruptive, I could hardpin the dependency, and we remove the hardpin and upgrade the version in #2925 |
|
@Poolitzer sounds okay. please add a corresponding note to #2925 |
Bibo-Joshi
left a comment
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.
The changes themself LGTM 👍 did you by any chance already test the examples? if not: do you want to before we merge or do we just do that later?
|
@Bibo-Joshi I didn't test it, I think that makes most sense before the first release. |
Co-authored-by: Harshil <37377066+harshil21@users.noreply.github.com>
|
Only thing I didn't run was passport, payment, and chatmemberbot. Rest all are working fine! Asyncio runs smooooth! |
Resolved issues in examples/rawapibot.py via DeepSource Autofix
Co-authored-by: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
|
looks good to me |
|
Tested chatmemberbot. Runs fine except for a but in |
|
Tested passport & payment example. For passports, I adjusted the html file a bit and found tdlib/telegram-bot-api#253 - which makes me want to stabilize If CI runs through & @harshil21 and @Poolitzer don't have anything to add, we can merge :) |
|
All good from my side |
|
Looks good to me as well |
Checklist for PRs
.. versionadded:: version,.. versionchanged:: versionor.. deprecated:: versionto the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)AUTHORS.rst(optional)If the PR contains API changes (otherwise, you can delete this passage)
New classes:
self._id_attrsand corresponding documentation__init__accepts**_kwargsAdded new shortcuts:
Chat&Userfor all methods that acceptchat/user_idMessagefor all methods that acceptchat_idandmessage_idMessageshortcuts: Addedquoteargument if methods acceptsreply_to_message_idCallbackQueryfor all methods that accept eitherchat_idandmessage_idorinline_message_idIf relevant:
telegram.constantsand shortcuts to them as class variablesREADME.rstandREADME_RAW.rst(including the badge), as well astelegram.constants.BOT_API_VERSIONtg.ext.Botfor new methods that either accept areply_markupin some form or have a return type that is/containstelegram.Message