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

result_image redundancy #153

Open
bjlittle opened this issue May 19, 2022 · 3 comments
Open

result_image redundancy #153

bjlittle opened this issue May 19, 2022 · 3 comments

Comments

@bjlittle
Copy link
Contributor

In the plugin.ImageComparison.compare_image_to_hash_library method, during hybrid-mode, the result_image appears to be unnecessarily copied into the summary dictionary from the outcome of plugin.ImageComparison.compare_image_to_baseline:

for k in ['image_status', 'baseline_image', 'diff_image',
'rms', 'tolerance', 'result_image']:
summary[k] = summary[k] or baseline_summary.get(k)

i.e., the summary['result_image'] has already been correctly set within plugin.ImageComparison.compare_image_to_hash:

# Save the figure for later summary (will be removed later if not needed)
test_image = (result_dir / "result.png").absolute()
fig.savefig(str(test_image), **savefig_kwargs)
summary['result_image'] = test_image.relative_to(self.results_dir).as_posix()

If this is the case, are you happy for me to remove this behaviour?

@dopplershift
Copy link
Contributor

Actually, it looks like compare_image_to_baseline duplicates the entire 3-line block of setting up the result image, which doesn't seem necessary at all?

@Cadair
Copy link
Contributor

Cadair commented May 20, 2022

I suspect that it could be tidied up, feel free to go for it.

I think it was @ConorMacBride who touched this section last?

@ConorMacBride
Copy link
Member

compare_image_to_baseline runs:

test_image = (result_dir / "result.png").absolute()
fig.savefig(str(test_image), **savefig_kwargs)
summary['result_image'] = test_image.relative_to(self.results_dir).as_posix()

So technically the result_image file generated in compare_image_to_hash_library is different to the new one generated by compare_image_to_baseline, although it should be at the same path. I just set it to copy the result_image to be safe, as both methods define their own (but equal) path.

These three lines are duplicated because the plugin can do baseline image comparison, hash library comparison or both (hybrid mode). But to prevent saving twice, maybe they should be moved to a separate method which only saves if it doesn't already exist.

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

No branches or pull requests

4 participants