-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
Fix: pytest warning - GioUnix was imported without specifying version #30535
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
Conversation
|
I'm of two minds on this. It's only a problem if you have PyGObject 3.54 and you set warnings to be errors. The latter is something we do in our tests explicitly, but I'm not sure if it's that common for people to do so. I also don't know that only |
|
Hi @QuLogic, Thanks for taking the time to review this and for your insightful comments. I completely agree with your assessment that this warning only becomes an error under specific conditions (PyGObject 3.54 + warnings as errors). You're right that most end-users may not encounter this. However, for developers who do run into this (like in our own CI pipeline or anyone who enables stricter warnings), the failing tests create immediate friction. It generates unnecessary noise and notifications, and can cause contributors to spend time wondering if they've introduced a new bug. My intention with this PR is to provide a quick, pragmatic fix to improve the immediate developer experience and keep our test suite clean. It's meant as a "stopgap" measure to resolve the current annoyance. I'm definitely open to a more comprehensive solution that addresses any other modules with similar import issues, as you alluded to. Perhaps we can merge this simple fix for now to solve the immediate problem, and I (or someone else) can open a new issue to track the larger task of auditing and applying more explicit version requirements across the board? What do you think of this approach? |
timhoffm
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.
This is a reasonable stopgap.
|
I created an issue and flagged it as release critical for 3.11 but merged for now. |
PR summary
Addresses issue #30525
PR checklist