Added type hints to Tests/helper.py #7733
Conversation
Tests/helper.py
Outdated
| assert a.mode == b.mode, msg or f"got mode {repr(a.mode)}, expected {repr(b.mode)}" | ||
| assert a.size == b.size, msg or f"got size {repr(a.size)}, expected {repr(b.size)}" | ||
| if a.tobytes() != b.tobytes(): | ||
| if HAS_UPLOADER: | ||
| if uploader: |
There was a problem hiding this comment.
This if is probably no longer needed since it is now checked in the upload function.
There was a problem hiding this comment.
If this condition was removed, then the logger.error on line 97 would always run, and similarly, the logger.exception on line 136 would always run.
There was a problem hiding this comment.
Perhaps it shouldn't run if there is no url returned?
If uploader = "show", that logger message is probably also undesirable.
url = upload(a, b)
if url:
logger.error("URL for test images: %s", url)There was a problem hiding this comment.
Ok, I've pushed f7701e6 for this.
Tests/helper.py
Outdated
| @@ -125,55 +130,68 @@ def assert_image_similar(a, b, epsilon, msg=None): | |||
| + f" average pixel value difference {ave_diff:.4f} > epsilon {epsilon:.4f}" | |||
| ) | |||
| except Exception as e: | |||
| if HAS_UPLOADER: | |||
| if uploader: | |||
There was a problem hiding this comment.
Ok, I've pushed f7701e6 for this.
There was a problem hiding this comment.
I also added some hints to this file yesterday but hadn't opened the PR yet :) The good news is we can combine these!
radarhere
force-pushed
the
type_hints_helper
branch
from
13f626f
to
f7701e6
Compare
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
for more information, see https://pre-commit.ci
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
| elif uploader == "aws": | ||
| return test_image_results.upload(a, b) |
There was a problem hiding this comment.
Out of scope for this PR, but if the AWS Lambda is long gone, shall we remove this stuff?
There was a problem hiding this comment.
Ok, I've created #7739 to remove it.
No description provided.