Skip to content
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

PR: Ignore PySide6 QSqlDatabase.exec DeprecationWarning #506

Merged

Conversation

juliangilbey
Copy link
Contributor

When running the package tests with PySide 6, the tests fail because PySide 6 gives a DeprecationWarning, and the pytest.ini settings turn this into an error. This PR ignores that specific error.

I tried using a warnings.catch_warnings() block in qtpy/QtSql.py around the deprecated functions, but pytest seems to ignore this. It might be worth adding such a block anyway.

@coveralls
Copy link

Coverage Status

coverage: 90.18%. remained the same
when pulling a26b0c6 on juliangilbey:suppress-deprecationwarning
into cd0b49b on spyder-ide:master.

@dalthviz dalthviz added this to the v2.4.3 milestone Dec 13, 2024
@dalthviz
Copy link
Member

Hi @juliangilbey thanks for the PR! So my guess is that doing something like:

def prevent_warn_exec(self, *args, **kwargs):
    with warnings.catch_warnings():
        warnings.simplefilter("ignore")
        self.exec(*args, **kwargs)

QSqlDatabase.exec_ = QSqlDatabase.exec = prevent_warn_exec

Didn't work, right? If that is the case then seems reasonable to just ignore the warning. Also what do you think @ccordoba12 ?

@dalthviz dalthviz changed the title Ignore PySide 6 DeprecationWarning PR: Ignore PySide6 QSqlDatabase.exec DeprecationWarning Dec 13, 2024
@dalthviz dalthviz changed the title PR: Ignore PySide6 QSqlDatabase.exec DeprecationWarning PR: Ignore PySide6 QSqlDatabase.exec DeprecationWarning Dec 13, 2024
@dalthviz
Copy link
Member

Note: More info on the deprecation at https://doc.qt.io/qt-6/qsqldatabase-obsolete.html#exec

@ccordoba12
Copy link
Member

Also what do you think @ccordoba12 ?

I think we shouldn't ignore the warning with the approach you proposed so that users actually see it and learn that calling exec is deprecated.

So, I'm ok with @juliangilbey's simple approach of ignoring the warning in our tests so they pass.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @juliangilbey!

@dalthviz dalthviz merged commit 1d2a1ea into spyder-ide:master Dec 16, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants