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

Ensure file is closed if it is opened by ImageQt.ImageQt #5260

merged 1 commit into from Mar 8, 2021


Copy link

@radarhere radarhere commented Feb 10, 2021

If the ImageQt.ImageQt class is passed a path, it opens the file - but it doesn't close it with close() or a context manager. This PR fixes that, closing im within _toqclass_helper.

_toqclass_helper previously returned im, but only for so that ImageQt.ImageQt could extract the size from it. So I've changed that to just return the size directly, and keep im within _toqclass_helper. This should be fine in terms of backwards compatibility, as the method is indicated to be for private use by the leading _.

@radarhere radarhere added the Qt label Feb 10, 2021
@radarhere radarhere force-pushed the imageqt_exclusive_fp branch from 4fc4f6d to a447e92 Feb 11, 2021
@radarhere radarhere force-pushed the imageqt_exclusive_fp branch from a447e92 to 254973f Feb 19, 2021
with pytest.warns(None) as record:

assert not record
Copy link

@hugovk hugovk Mar 7, 2021

Do you know why this is missing coverage? We should have some Qt at least on some CI?

(Similarly for #5260)

Copy link

@nulano nulano Mar 7, 2021

It looks like Codecov only received AppVeyor coverage for this PR:

There is PyQt5 in the Arch container (among others), its coverage is reported here:

There are some errors reported:

==> GitHub Actions detected.
->  Issue detecting commit SHA. Please run actions/checkout with fetch-depth > 1 or set to 0
    project root: .
    Yaml found at: codecov.yml

->  Pinging Codecov
{'detail': ErrorDetail(string='Actions workflow run is stale', code='not_found')}

Copy link
Member Author

@radarhere radarhere Mar 8, 2021

Hmm. I tried rebasing in hopes of getting more reports, but I only got one more.

Copy link

@hugovk hugovk Mar 8, 2021

Well we get some coverage here now:

and shows a load of GHA builds too. Good enough for this PR, let's keep an eye on Codecov... Thanks!

hugovk approved these changes Mar 7, 2021
@radarhere radarhere force-pushed the imageqt_exclusive_fp branch from 254973f to e7f5bb1 Mar 8, 2021
@hugovk hugovk merged commit d9e4424 into python-pillow:master Mar 8, 2021
48 of 50 checks passed
@radarhere radarhere deleted the imageqt_exclusive_fp branch Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants