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
merged 1 commit into from Mar 8, 2021

Conversation

radarhere
Copy link
Member

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 Qt for Python, PyQt, PySide label Feb 10, 2021
with pytest.warns(None) as record:
ImageQt.ImageQt("Tests/images/hopper.gif")

assert not record
Copy link
Member

@hugovk hugovk Mar 7, 2021

Choose a reason for hiding this comment

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

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

(Similarly for #5260)

Copy link
Contributor

@nulano nulano Mar 7, 2021

Choose a reason for hiding this comment

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

It looks like Codecov only received AppVeyor coverage for this PR: https://codecov.io/gh/python-pillow/Pillow/commit/254973f8a035275c342e408e6ec883e5b2d2ee47/build

There is PyQt5 in the Arch container (among others), its coverage is reported here: https://codecov.io/github/python-pillow/Pillow/commit/d5f5ae68a8f758e53188903a01f43f693bff3220

There are some errors reported:

https://github.com/python-pillow/Pillow/pull/5260/checks?check_run_id=1933751967#step:8:21

==> 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

https://github.com/python-pillow/Pillow/pull/5260/checks?check_run_id=1933751943#step:15:763

->  Pinging Codecov
https://codecov.io/upload/v4?package=bash-20210129-7c25fce&token=secret&branch=imageqt_exclusive_fp&commit=d5f5ae68a8f758e53188903a01f43f693bff3220&build=580803277&build_url=http%3A%2F%2Fgithub.com%2Fpython-pillow%2FPillow%2Factions%2Fruns%2F580803277&name=ubuntu-latest%20Python%203.9&tag=&slug=python-pillow%2FPillow&service=github-actions&flags=GHA_Ubuntu&pr=5260&job=Test&cmd_args=F
{'detail': ErrorDetail(string='Actions workflow run is stale', code='not_found')}
404

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

@hugovk hugovk Mar 8, 2021

Choose a reason for hiding this comment

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

Well we get some coverage here now: https://codecov.io/gh/python-pillow/Pillow/src/dc0db43d35b081ad5f0f6266be79c0355d284f3b/Tests/test_imageqt.py

and https://codecov.io/gh/python-pillow/Pillow/commit/dc0db43d35b081ad5f0f6266be79c0355d284f3b/build shows a load of GHA builds too. Good enough for this PR, let's keep an eye on Codecov... Thanks!

@hugovk hugovk merged commit d9e4424 into python-pillow:master Mar 8, 2021
@radarhere radarhere deleted the imageqt_exclusive_fp branch March 8, 2021 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Qt Qt for Python, PyQt, PySide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants