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

Address a Qt DeprecationWarning and ResourceWarnings #964

Merged
merged 2 commits into from
Jul 1, 2021

Conversation

rahulporuri
Copy link
Contributor

This PR addresses a specific Qt DeprecationWarning - replacing the deprecated font_metrics.width method with the
recommended font_metrics.horizontalAdvance method. This PR also fixes ResourceWarnings by properly closing temporary files that a test had created/opened.

Ref https://doc.qt.io/qt-5/qfontmetrics-obsolete.html#width and https://doc.qt.io/qt-5/qfontmetrics.html#horizontalAdvance

by replacing the deprecated font_metrics.width method with the
recommended font_metrics.horizontalAdvance method and by properly
closing temporary files that a test had created/opened

	modified:   pyface/ui/qt4/console/console_widget.py
	modified:   pyface/ui/qt4/tasks/tests/test_split_editor_area_pane.py
Copy link
Contributor

@aaronayres35 aaronayres35 left a comment

Choose a reason for hiding this comment

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

LGTM

For some reason I was not seeing the deprecation warnings on master, but nonetheless the changes LGTM. Also I confirmed I no longer see the ResourceWarnings

@rahulporuri
Copy link
Contributor Author

For some reason I was not seeing the deprecation warnings on master

I first noticed them when I ran the testsuite on Python 3.8 - not Python 3.6 that we use by default.

@rahulporuri rahulporuri merged commit 166156a into master Jul 1, 2021
@rahulporuri rahulporuri deleted the fix/deprecation-resource-warnings branch July 1, 2021 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants