Skip to content

Improve Warning Categories & Stacklevels #3674

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.

Sign up for GitHub

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

Jump to bottom
Merged
Bibo-Joshi merged 10 commits into from
Apr 27, 2023
Merged

Improve Warning Categories & Stacklevels #3674

Bibo-Joshi merged 10 commits into from
Apr 27, 2023

Conversation

@Bibo-Joshi
Copy link
Member

@Bibo-Joshi Bibo-Joshi commented Apr 21, 2023 •
edited
Loading

  • Fixed a few warning categories that were missed in API 6.6.
  • Unifies warning handling in Bot and ExtBot, ensuring that the stacklevel is correct for both
  • adds missing tests & modifies existing ones

TODO:

  • Update all warning tests to also check the category
  • Update tests to double check stacklevel in ExtBot vs Bot

@Bibo-Joshi Bibo-Joshi self-assigned this Apr 21, 2023
@Bibo-Joshi Bibo-Joshi added enhancement ⚙️ tests affected functionality: tests labels Apr 21, 2023
harshil21
harshil21 reviewed Apr 21, 2023
View reviewed changes
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

nice catches

@Bibo-Joshi Bibo-Joshi marked this pull request as ready for review April 22, 2023 21:13
lemontree210
lemontree210 requested changes Apr 23, 2023
View reviewed changes
Copy link
Member

@lemontree210 lemontree210 left a comment

Choose a reason for hiding this comment

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

I'm obviously out of my depth here, but one typo I did catch! :)

@Bibo-Joshi
Copy link
Member Author

Bibo-Joshi commented Apr 23, 2023

Just a related note: stackleveling will get way more fun soon :D https://github.com/python-telegram-bot/python-telegram-bot/projects/8#card-88970053

@Bibo-Joshi
Review:
22a1883 
* Fix typo
* Make sure to only use the local information on stacklevel
harshil21
harshil21 reviewed Apr 24, 2023
View reviewed changes
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

much more extensive testing, I like it

@Bibo-Joshi Bibo-Joshi changed the title Improve Warning Categories Improve Warning Categories & Stacklevels Apr 24, 2023
@Bibo-Joshi
Copy link
Member Author

Bibo-Joshi commented Apr 24, 2023

While working on #3673 I noticed that the subscription access to attributes of TelegramObject can mess with the stacklevel. Because message["text_markdown"] calls message.__getitem__("text_markdown") that's one more function call meaning that the warning will originate from _telegramobject.py instead of the user code. I guess the only workaround for that would be to catch & reraise all warnings in __getitem__ and that's not really clean … IMO the use case is small enough to ignore. Just wanted to note it in the context of this PR :)

@harshil21
Copy link
Member

harshil21 commented Apr 25, 2023

I wonder if there is a way to dynamically inspect the stacklevel and pass the correct value for cases like those...

@Bibo-Joshi
Copy link
Member Author

Bibo-Joshi commented Apr 25, 2023

@harshil21 I guess there is inspect.stack. I'm not so sure if I want to get the inspect module involved for this though ...

harshil21
harshil21 approved these changes Apr 26, 2023
View reviewed changes
Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

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

lgtm

@Bibo-Joshi
Copy link
Member Author

Bibo-Joshi commented Apr 27, 2023

Test failures are unrelated - see #3685 and #3673. Merging.

@Bibo-Joshi Bibo-Joshi merged commit 3f444da into master Apr 27, 2023
@Bibo-Joshi Bibo-Joshi deleted the warnings-categories branch April 27, 2023 20:36
@Bibo-Joshi Bibo-Joshi mentioned this pull request Apr 27, 2023
Merged
15 tasks
@github-actions github-actions bot locked and limited conversation to collaborators May 5, 2023
@Bibo-Joshi Bibo-Joshi added 🔌 enhancement pr description: enhancement and removed enhancement labels Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lemontree210 lemontree210 lemontree210 requested changes

@harshil21 harshil21 harshil21 approved these changes

Assignees

@Bibo-Joshi Bibo-Joshi

Labels

🔌 enhancement pr description: enhancement ⚙️ tests affected functionality: tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Bibo-Joshi @harshil21 @lemontree210